carverauto/serviceradar

Fork 0

Updates/post tenant cleanup fixes #2682

Merged

mfreeman451 merged 8 commits from refs/pull/2682/head into staging

2026-01-17 19:54:57 +00:00

mfreeman451 commented

2026-01-17 17:53:59 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2321
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2321
Original created: 2026-01-17T17:53:59Z
Original updated: 2026-01-17T19:54:59Z
Original head: carverauto/serviceradar:updates/post-tenant-cleanup-fixes
Original base: staging
Original merged: 2026-01-17T19:54:57Z by @mfreeman451

User description

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?

PR Type

Enhancement, Bug fix

Description

Remove Go-based CNPG migrations; consolidate schema to Ash migration
Fix Oban tables created in wrong schema by removing explicit prefix
Add Oban schema to consolidated Ash migration for idempotent startup
Fix Horde supervisor naming conflict in ProcessRegistry
Make TLS client certificates optional in CNPG connections
Update Helm/Compose to use core-elx migrations instead of cnpg-migrate tool

Diagram Walkthrough

flowchart LR
  A["Go cnpg-migrate tool"] -->|removed| B["Ash rebuild migration"]
  C["Oban prefix explicit"] -->|fixed| D["Oban uses search_path"]
  E["ProcessRegistry.Supervisor"] -->|renamed| F["ProcessSupervisor"]
  G["TLS certs required"] -->|made optional| H["TLS certs conditional"]
  B --> I["Single schema source"]
  D --> I
  F --> I
  H --> I

File Walkthrough

Relevant files

Bug fix

8 files

20260117090000_rebuild_schema.exs `Remove prefix from Oban migrations and reorder gateways/partitions`	+36/-66
application.ex `Guard ProcessRegistry startup to prevent duplicate initialization`	+7/-1
process_registry.ex `Rename supervisor to ProcessSupervisor to avoid naming conflicts`	+1/-1
db.go `Remove Go-based CNPG migrations and shouldRunDBMigrations function`	+2/-30
cnpg_pool.go `Make TLS client certificates optional in CNPG connections`	+11/-5
config.go `Fix TLS path normalization to handle empty paths correctly`	+5/-4
config.go `Make NATS credentials optional when using mTLS authentication`	+7/-1
Dockerfile.web-ng `Remove ecto.migrate from web-ng startup command`	+1/-1

Configuration changes

4 files

runtime.exs `Add platform schema prefix to Oban configuration`	+1/-0
config.exs `Add platform schema prefix to Oban configuration`	+1/-0
runtime.exs `Add platform schema prefix to Oban configuration`	+1/-0
values.yaml `Enable agent gateway and database migrations by default`	+3/-1

Documentation

17 files

ash_scope.ex `Update documentation comment for schema override`	+1/-1
errors.go `Update error message from schema to database initialization`	+1/-1
agents.md `Replace cnpg-migrate documentation with Ash migration instructions`	+15/-67
otel.md `Update OTEL metrics retention documentation to reference Ash migration`	+1/-1
cnpg-monitoring.md `Update trigram index documentation to reference Ash migration`	+5/-10
age-graph-schema.md `Update AGE graph schema documentation to reference Ash migration`	+1/-1
srql-language-reference.md `Update SRQL schema references to point to Ash rebuild migration`	+4/-4
docker-compose-login-500.md `Replace cnpg-migrate with mix ash.migrate in troubleshooting guide`	+3/-3
identity-backfill.md `Update prerequisites to reference Ash migrations instead of` `cnpg-migrate`	+1/-1
proposal.md `New proposal document for fixing Helm deployment bootstrap issues`	+97/-0
tasks.md `New tasks document for Helm deployment bootstrap fixes`	+174/-0
spec.md `New specification for Helm deployment requirements`	+87/-0
proposal.md `New proposal for stabilizing Docker Compose stack startup`	+17/-0
design.md `New design document for Docker Compose stack stabilization`	+39/-0
tasks.md `New tasks document for Docker Compose stack fixes`	+22/-0
spec.md `New specification for Docker Compose stack requirements`	+38/-0
tasks.md `Update task status for schema isolation verification`	+10/-4

Enhancement

9 files

reset-cnpg.sh `Replace cnpg-migrate with core-elx startup migrations`	+1/-3
docker-compose.yml `Remove cnpg-migrate service and update dependencies`	+3/-29
core.yaml `Make CNPG client certs conditional and add NATS creds mounting`	+17/-0
web.yaml `Make CNPG client certificates conditional on configuration`	+2/-0
db-event-writer.yaml `Enable database migrations by default in db-event-writer`	+1/-1
db-event-writer-config.yaml `Make TLS client certificates conditional in db-event-writer config`	+3/-0
spire-server.yaml `Add conditional client certificate configuration for SPIRE`	+5/-0
spire-postgres.yaml `Add default pg_hba rules for SSL without client certificates`	+9/-1
cnpg-client-ca-secret-job.yaml `New job to create CNPG client CA secret from certificate data`	+76/-0

Additional files

20 files

MODULE.bazel	+0/-3
Makefile	+0/-4
BUILD.bazel	+0/-20
main.go	+0/-264
BUILD.bazel	+0/-2
service-registry-design.md	+0/-1057
service-registry-status.md	+0/-449
repo.ex	+0/-11
serviceradar-tools.yaml	+0/-1
BUILD.bazel	+0/-4
00000000000001_schema.up.sql	+0/-462
cnpg_migrate.go	+0/-129
cnpg_registry.go	+0/-639
cnpg_registry_test.go	+0/-142
gateways.go	+0/-110
services.go	+0/-35
service_models.go	+0/-147
service_registry.go	+0/-992
service_registry_queries.go	+0/-715
service_registry_queries_test.go	+0/-387

Imported from GitHub pull request. Original GitHub pull request: #2321 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2321 Original created: 2026-01-17T17:53:59Z Original updated: 2026-01-17T19:54:59Z Original head: carverauto/serviceradar:updates/post-tenant-cleanup-fixes Original base: staging Original merged: 2026-01-17T19:54:57Z by @mfreeman451 --- ### **User description** ## 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? ___ ### **PR Type** Enhancement, Bug fix ___ ### **Description** - Remove Go-based CNPG migrations; consolidate schema to Ash migration - Fix Oban tables created in wrong schema by removing explicit prefix - Add Oban schema to consolidated Ash migration for idempotent startup - Fix Horde supervisor naming conflict in ProcessRegistry - Make TLS client certificates optional in CNPG connections - Update Helm/Compose to use core-elx migrations instead of cnpg-migrate tool ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Go cnpg-migrate tool"] -->|removed| B["Ash rebuild migration"] C["Oban prefix explicit"] -->|fixed| D["Oban uses search_path"] E["ProcessRegistry.Supervisor"] -->|renamed| F["ProcessSupervisor"] G["TLS certs required"] -->|made optional| H["TLS certs conditional"] B --> I["Single schema source"] D --> I F --> I H --> I ``` <details><summary><h3>File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td>Bug fix</td><td><details><summary>8 files</summary><table> <tr> <td>20260117090000_rebuild_schema.exs<dd><code>Remove prefix from Oban migrations and reorder gateways/partitions</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-bd2f961d2daeff2221ce5121ca6efcbfc54452980e8827de43f70d2f0137c9fc">+36/-66</a>  </td> </tr> <tr> <td>application.ex<dd><code>Guard ProcessRegistry startup to prevent duplicate initialization</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-a9ffbf400b7f9b22cd8980c41286c54fe373f1f1a8684bb6a344a5fb39b178d0">+7/-1</a>      </td> </tr> <tr> <td>process_registry.ex<dd><code>Rename supervisor to ProcessSupervisor to avoid naming conflicts</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-02d0807ca6ef162886993bd04fb1c058240143ad79c49af03cda12f3fa63b6df">+1/-1</a>      </td> </tr> <tr> <td>db.go<dd><code>Remove Go-based CNPG migrations and shouldRunDBMigrations function</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-5da3684806835246d262230050593f460b12b6c0e3966df174e6061be0e9e575">+2/-30</a>    </td> </tr> <tr> <td>cnpg_pool.go<dd><code>Make TLS client certificates optional in CNPG connections</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-c5919d757d10f4573450733578c3072c7a67203c2a205ac884c084431460268b">+11/-5</a>    </td> </tr> <tr> <td>config.go<dd><code>Fix TLS path normalization to handle empty paths correctly</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-a3d824da3c42420cd5cbb0a4a2c0e7b5bfddd819652788a0596d195dc6e31fa5">+5/-4</a>      </td> </tr> <tr> <td>config.go<dd><code>Make NATS credentials optional when using mTLS authentication</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-241fb78182e99079654727a4140db640d348e0e1e813fd35d32c99b2d1ff78b3">+7/-1</a>      </td> </tr> <tr> <td>Dockerfile.web-ng<dd><code>Remove ecto.migrate from web-ng startup command</code>                    </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-92d43af1965575d56c3380ecc8a81024aac2ff36f039ec2d3839e9fc7852bc10">+1/-1</a>      </td> </tr> </table></details></td></tr><tr><td>Configuration changes</td><td><details><summary>4 files</summary><table> <tr> <td>runtime.exs<dd><code>Add platform schema prefix to Oban configuration</code>                  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-0d835809f7c8d80e73d1de1ced438bbe735892a0b4af1a2b650eff6ce1699d43">+1/-0</a>      </td> </tr> <tr> <td>config.exs<dd><code>Add platform schema prefix to Oban configuration</code>                  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-42b3888cc53d9dcf4ebc45ec7fffb2c672b129bffe763b6c76de58e4678a13a8">+1/-0</a>      </td> </tr> <tr> <td>runtime.exs<dd><code>Add platform schema prefix to Oban configuration</code>                  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-f1093c1a9d74ba3cd35f31e2c50dec60b4fb9c241fc5ff9f62bcbcc0559691ae">+1/-0</a>      </td> </tr> <tr> <td>values.yaml<dd><code>Enable agent gateway and database migrations by default</code>    </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-d4449c7cb70362554b274f81eae5a4b81a8e81df494282e383d1b7ea3871c452">+3/-1</a>      </td> </tr> </table></details></td></tr><tr><td>Documentation</td><td><details><summary>17 files</summary><table> <tr> <td>ash_scope.ex<dd><code>Update documentation comment for schema override</code>                  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-c3fa8bce36351e00e6dc4ea38c6ed9a55cd6f872b406dc1bd2c45725052bfaf6">+1/-1</a>      </td> </tr> <tr> <td>errors.go<dd><code>Update error message from schema to database initialization</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-993d3cbb9129e2dbe72c2785718929ee4a7dc9e3974b73f717f8c83eede44957">+1/-1</a>      </td> </tr> <tr> <td>agents.md<dd><code>Replace cnpg-migrate documentation with Ash migration instructions</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-af8d04277f2353629065b0cc5fad3e44bd3e7c20339bd125e0812104bdbeff28">+15/-67</a>  </td> </tr> <tr> <td>otel.md<dd><code>Update OTEL metrics retention documentation to reference Ash migration</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-498eac46d1b44ca23346a9ef5edc4f38030fe37ce7313e691a0644e9deb381f4">+1/-1</a>      </td> </tr> <tr> <td>cnpg-monitoring.md<dd><code>Update trigram index documentation to reference Ash migration</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-8c105f73fd3323743d06525e84c5d0c686b447742dca1a5cf2776b1afc9b8ba2">+5/-10</a>    </td> </tr> <tr> <td>age-graph-schema.md<dd><code>Update AGE graph schema documentation to reference Ash migration</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-2dc9168ae343d7818dc9010b57aaeae3671e24e4a3d1fb640d907427e649a215">+1/-1</a>      </td> </tr> <tr> <td>srql-language-reference.md<dd><code>Update SRQL schema references to point to Ash rebuild migration</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-7fa04d96755318e393180aaae41cb1374c1da332b3fca76c29e443fd7146ef5f">+4/-4</a>      </td> </tr> <tr> <td>docker-compose-login-500.md<dd><code>Replace cnpg-migrate with mix ash.migrate in troubleshooting guide</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-d3ec1ba25f119cff4efcb5767a1afe923b92fd5727453c6276030bc8e9923f05">+3/-3</a>      </td> </tr> <tr> <td>identity-backfill.md<dd><code>Update prerequisites to reference Ash migrations instead of </code> <code>cnpg-migrate</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-5b2319b033f8fcf8e51b679b1a2ba132bcab16b904784b052cd170a3222866aa">+1/-1</a>      </td> </tr> <tr> <td>proposal.md<dd><code>New proposal document for fixing Helm deployment bootstrap issues</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-4a3d2e37167f81e08589cd27a9101da64f97d48a4653948ffae8db6c25323f13">+97/-0</a>    </td> </tr> <tr> <td>tasks.md<dd><code>New tasks document for Helm deployment bootstrap fixes</code>      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-dd91eea43b3d117b7392ede2daf96b9c766173e5617107ce0e9f702c033a7bdc">+174/-0</a>  </td> </tr> <tr> <td>spec.md<dd><code>New specification for Helm deployment requirements</code>              </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-8b72445550020273f20bc67af7c9b539d7bb1d3661afbb007a39de7d22b377df">+87/-0</a>    </td> </tr> <tr> <td>proposal.md<dd><code>New proposal for stabilizing Docker Compose stack startup</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-1060c826989381193044774c23249af7c08671a5acea6d5f2c6b80ec5b8200bb">+17/-0</a>    </td> </tr> <tr> <td>design.md<dd><code>New design document for Docker Compose stack stabilization</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-020ff53d12ea6185b9dab73cd206cb985e261d4dd93811d3ffc697db32399401">+39/-0</a>    </td> </tr> <tr> <td>tasks.md<dd><code>New tasks document for Docker Compose stack fixes</code>                </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-1fd30648dc5b0fdadefd5c463742192e17a8b07c703b732e3bd457f933ac5c27">+22/-0</a>    </td> </tr> <tr> <td>spec.md<dd><code>New specification for Docker Compose stack requirements</code>    </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-4cff162e54461d7a265a3d56fbfe3ec17b68f65afc18219a49a9d29bb1f315e4">+38/-0</a>    </td> </tr> <tr> <td>tasks.md<dd><code>Update task status for schema isolation verification</code>          </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-6eeec38b40128b8459a553f6a84532940a02cdcc430b14cfc8695e21a3fd90c9">+10/-4</a>    </td> </tr> </table></details></td></tr><tr><td>Enhancement</td><td><details><summary>9 files</summary><table> <tr> <td>reset-cnpg.sh<dd><code>Replace cnpg-migrate with core-elx startup migrations</code>        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-b150c895e524739f2e3d478e2384097fc7a7cf5abbcff278c14d82aa0a62b572">+1/-3</a>      </td> </tr> <tr> <td>docker-compose.yml<dd><code>Remove cnpg-migrate service and update dependencies</code>            </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-e45e45baeda1c1e73482975a664062aa56f20c03dd9d64a827aba57775bed0d3">+3/-29</a>    </td> </tr> <tr> <td>core.yaml<dd><code>Make CNPG client certs conditional and add NATS creds mounting</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-06ab387d2c169d82a1de28b5e66c86f0417bd81b82a96246d0a2da8bfaa8d224">+17/-0</a>    </td> </tr> <tr> <td>web.yaml<dd><code>Make CNPG client certificates conditional on configuration</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-6e0340bf29070d4b88c305a585cf278672914f74f95dbcfde215da3fcbd0f562">+2/-0</a>      </td> </tr> <tr> <td>db-event-writer.yaml<dd><code>Enable database migrations by default in db-event-writer</code>  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-e4f899d11e5720f7049aa6fd632bd6993739410051bf65bc6fc8469739e5d2e4">+1/-1</a>      </td> </tr> <tr> <td>db-event-writer-config.yaml<dd><code>Make TLS client certificates conditional in db-event-writer config</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-4cce0ab31bec3428ffae6701d20ca14b0f27a1e8a810ba1c7388e5c7860c3254">+3/-0</a>      </td> </tr> <tr> <td>spire-server.yaml<dd><code>Add conditional client certificate configuration for SPIRE</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-7959f7b987adcd56306fe5ddf31fed367dd9bb995f9b82a8fa53553dac8e7077">+5/-0</a>      </td> </tr> <tr> <td>spire-postgres.yaml<dd><code>Add default pg_hba rules for SSL without client certificates</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-1ca173bb2b8cb20ea64a4f1901850a229c0c9f40286f074e00b3301be99299bd">+9/-1</a>      </td> </tr> <tr> <td>cnpg-client-ca-secret-job.yaml<dd><code>New job to create CNPG client CA secret from certificate data</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-09fdf0d1b7a1d044a233516added81d55d6a50127e68504ac6e507d494ed12b6">+76/-0</a>    </td> </tr> </table></details></td></tr><tr><td>Additional files</td><td><details><summary>20 files</summary><table> <tr> <td>MODULE.bazel</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-6136fc12446089c3db7360e923203dd114b6a1466252e71667c6791c20fe6bdc">+0/-3</a>      </td> </tr> <tr> <td>Makefile</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52">+0/-4</a>      </td> </tr> <tr> <td>BUILD.bazel</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-0d373909abab60bd29fd250496c1fd02684991636edc7d4bb1b8ace068ff70a9">+0/-20</a>    </td> </tr> <tr> <td>main.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-93eee0002cd036549d4ef0161788fbe4f4382eb266cc83b89517953b7448803e">+0/-264</a>  </td> </tr> <tr> <td>BUILD.bazel</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-0e4db31c224a8f72ae8e870a849e38a59d74a2c7f7b04347b0b3eb07e20c5a80">+0/-2</a>      </td> </tr> <tr> <td>service-registry-design.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-eeff62de7c13c07e98f187ab6b5173585e47ab26969ddae687e69783483116fa">+0/-1057</a></td> </tr> <tr> <td>service-registry-status.md</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-14b25655121cc9d193e18e37912ff77cc784ac513d53ec8240b45d440bd72575">+0/-449</a>  </td> </tr> <tr> <td>repo.ex</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-07a1d27a8e8c0f459cb06766961c8cf9d914a9ed04a539b98c6110f439590df1">+0/-11</a>    </td> </tr> <tr> <td>serviceradar-tools.yaml</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-89330d07cf9cc5953c8f6a96c9698450d59baceeeff2aa265b0e92c1b3c21852">+0/-1</a>      </td> </tr> <tr> <td>BUILD.bazel</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-adb10902cc707bef3ae70e4f5cdf15c34842727adc0f02fcf2dac044a3bfe004">+0/-4</a>      </td> </tr> <tr> <td>00000000000001_schema.up.sql</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-6a6f36e8670865c966dffe617122eec32dd034277ea0832973c5e79f359b7805">+0/-462</a>  </td> </tr> <tr> <td>cnpg_migrate.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-86f32b7dd2b2fce5abdd0f0fa3d228b5ae39c5d46154163ecd196456192ab13d">+0/-129</a>  </td> </tr> <tr> <td>cnpg_registry.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-e31b8b854d9ba774d2f3ed9899b1f5902462cd0e63990a2898dcaf9bc171d571">+0/-639</a>  </td> </tr> <tr> <td>cnpg_registry_test.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-e38716ee5b3614089e00e851d22d2c61e9934ebcc2b44b7aa4b79623251af869">+0/-142</a>  </td> </tr> <tr> <td>gateways.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-fce79070359ad45933dc0cc9fcc35c099f8fbaf538e1eee37713d3e55816c5a4">+0/-110</a>  </td> </tr> <tr> <td>services.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-08bcc70ff62d6bd79ef5139eb844e853b9b999a2ebde0e75f8e546afb44ff022">+0/-35</a>    </td> </tr> <tr> <td>service_models.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-3f9834ff929a8557454ae9d65656dc2a25134184e36c75afa0e47821e9b40894">+0/-147</a>  </td> </tr> <tr> <td>service_registry.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-d6d6d09a58edde934a1d3f571ff5ce02f0475c5e85ecdec27888892aff8d9d1d">+0/-992</a>  </td> </tr> <tr> <td>service_registry_queries.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-3591e8646384be68811f862b986342253cabc23176c6f0e09996453baf88a2e9">+0/-715</a>  </td> </tr> <tr> <td>service_registry_queries_test.go</td> <td><a href="https://github.com/carverauto/serviceradar/pull/2321/files#diff-6375daf4a77f18366326db1ac818f613eea7735f4bdc23c886274edc58826656">+0/-387</a>  </td> </tr> </table></details></td></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2026-01-17 17:54:52 +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/2321#issuecomment-3764160086
Original created: 2026-01-17T17:54:52Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Unpinned container image Description: The Job uses an unpinned image tag `bitnami/kubectl:latest`, which can silently change over time and introduces a supply-chain risk (unexpected image contents or compromised upstream tag). cnpg-client-ca-secret-job.yaml [24-24] Referred Code `image: bitnami/kubectl:latest command: ["/bin/sh"]`
	TLS client auth relaxation Description: When `requireClientCert` is false, the generated `pg_hba` rules allow `hostssl ...` `scram-sha-256` without `clientcert=verify-ca`, which weakens database access controls from mutual TLS to password-only TLS and can enable unauthorized CNPG access if credentials are obtained (this pattern is reinforced by making client cert env vars conditional in other Helm templates). spire-postgres.yaml [142-150] Referred Code `pg_hba: - "hostnossl all all all reject" - "hostssl all all all scram-sha-256 clientcert=verify-ca" {{- else }} # Allow SSL connections with password auth (no client cert required) pg_hba: - "hostnossl all all all reject" - "hostssl all all all scram-sha-256" {{- end }}`
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Audit coverage unclear: The PR primarily changes database bootstrap/migrations behavior and does not show any explicit audit logging additions, so it is not possible to verify from this diff whether all critical actions are consistently logged with user context. Referred Code // New creates a new CNPG-backed database connection. func New(ctx context.Context, config *models.CoreServiceConfig, log logger.Logger) (Service, error) { if config == nil { return nil, fmt.Errorf("%w: database configuration missing", ErrFailedOpenDB) } cnpgPool, err := newCNPGPool(ctx, config, log) if err != nil { return nil, err } if cnpgPool == nil { return nil, fmt.Errorf("%w: CNPG configuration not provided", ErrFailedOpenDB) } db := &DB{ pgPool: cnpgPool, executor: cnpgPool, // Default to pool logger: log, deviceUpdatesMu: &sync.Mutex{}, } ... (clipped 3 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Error exposure unknown: The diff adjusts TLS validation and connection URL building but does not show the call sites that surface these errors, so it cannot be confirmed whether detailed internal errors are only logged internally and not returned to end-users. Referred Code query.Set("sslmode", sslMode) if cfg.TLS != nil { caFile := resolveCNPGTLSPath(cfg, cfg.TLS.CAFile) // CA file is required for TLS verification (verify-ca, verify-full modes) if caFile == "" { return url.URL{}, ErrCNPGLackingTLSFiles } query.Set("sslrootcert", caFile) // Client cert and key are optional (only required if server demands client auth) certFile := resolveCNPGTLSPath(cfg, cfg.TLS.CertFile) keyFile := resolveCNPGTLSPath(cfg, cfg.TLS.KeyFile) if certFile != "" && keyFile != "" { query.Set("sslcert", certFile) query.Set("sslkey", keyFile) } } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Logging format unknown: The PR adds/updates operational log output and configuration, but the diff does not provide enough evidence about structured logging or whether runtime logs could include sensitive data. Referred Code `echo "⏳ Waiting for CNPG pods to become Ready..." kubectl wait --for=condition=Ready --timeout=600s pod -l "${SELECTOR}" -n "${NAMESPACE}" echo "🔁 Ash migrations are applied by core-elx on startup (SERVICERADAR_CORE_RUN_MIGRATIONS=true)." echo "♻️ Restarting core services so they reconnect to the fresh database..." for deployment in serviceradar-core serviceradar-datasvc \` Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: TLS mode handling: Client certificate handling is made optional and CA enforcement is changed, but without the full context of `sslmode` derivation and config sources it requires verification that all TLS modes and external config inputs are validated and enforced as intended. Referred Code query.Set("sslmode", sslMode) if cfg.TLS != nil { caFile := resolveCNPGTLSPath(cfg, cfg.TLS.CAFile) // CA file is required for TLS verification (verify-ca, verify-full modes) if caFile == "" { return url.URL{}, ErrCNPGLackingTLSFiles } query.Set("sslrootcert", caFile) // Client cert and key are optional (only required if server demands client auth) certFile := resolveCNPGTLSPath(cfg, cfg.TLS.CertFile) keyFile := resolveCNPGTLSPath(cfg, cfg.TLS.KeyFile) if certFile != "" && keyFile != "" { query.Set("sslcert", certFile) query.Set("sslkey", keyFile) } } ... (clipped 2 lines) Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2321#issuecomment-3764160086 Original created: 2026-01-17T17:54:52Z --- ## PR Compliance Guide 🔍  Below is a summary of compliance checks for this PR: <table><tbody><tr><td colspan='2'>Security Compliance</td></tr> <tr><td rowspan=2>⚪</td> <td><details><summary>Unpinned container image </summary> Description: The Job uses an unpinned image tag <code>bitnami/kubectl:latest</code>, which can silently change over time and introduces a supply-chain risk (unexpected image contents or compromised upstream tag). <a href='https://github.com/carverauto/serviceradar/pull/2321/files#diff-09fdf0d1b7a1d044a233516added81d55d6a50127e68504ac6e507d494ed12b6R24-R24'>cnpg-client-ca-secret-job.yaml [24-24]</a> <details open><summary>Referred Code</summary> ```yaml image: bitnami/kubectl:latest command: ["/bin/sh"] ``` </details></details></td></tr> <tr><td><details><summary>TLS client auth relaxation </summary> Description: When <code>requireClientCert</code> is false, the generated <code>pg_hba</code> rules allow <code>hostssl ... </code> <code>scram-sha-256</code> without <code>clientcert=verify-ca</code>, which weakens database access controls from mutual TLS to password-only TLS and can enable unauthorized CNPG access if credentials are obtained (this pattern is reinforced by making client cert env vars conditional in other Helm templates). <a href='https://github.com/carverauto/serviceradar/pull/2321/files#diff-1ca173bb2b8cb20ea64a4f1901850a229c0c9f40286f074e00b3301be99299bdR142-R150'>spire-postgres.yaml [142-150]</a> <details open><summary>Referred Code</summary> ```yaml pg_hba: - "hostnossl all all all reject" - "hostssl all all all scram-sha-256 clientcert=verify-ca" {{- else }} # Allow SSL connections with password auth (no client cert required) pg_hba: - "hostnossl all all all reject" - "hostssl all all all scram-sha-256" {{- end }} ``` </details></details></td></tr> <tr><td colspan='2'>Ticket Compliance</td></tr> <tr><td>⚪</td><td><details><summary>🎫 No ticket provided </summary> - [ ] Create ticket/issue  </details></td></tr> <tr><td colspan='2'>Codebase Duplication Compliance</td></tr> <tr><td>⚪</td><td><details><summary>Codebase context is not defined </summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'>Custom Compliance</td></tr> <tr><td rowspan=2>🟢</td><td> <details><summary>Generic: Meaningful Naming and Self-Documenting Code</summary> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code self-documenting **Status:** Passed > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary>Generic: Robust Error Handling and Edge Case Management</summary> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful degradation **Status:** Passed > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=4>⚪</td> <td><details> <summary>Generic: Comprehensive Audit Trails</summary> **Objective:** To create a detailed and reliable record of critical system actions for security analysis and compliance. **Status:** <a href='https://github.com/carverauto/serviceradar/pull/2321/files#diff-5da3684806835246d262230050593f460b12b6c0e3966df174e6061be0e9e575R65-R88'>Audit coverage unclear</a>: The PR primarily changes database bootstrap/migrations behavior and does not show any explicit audit logging additions, so it is not possible to verify from this diff whether all critical actions are consistently logged with user context. <details open><summary>Referred Code</summary> ```go // New creates a new CNPG-backed database connection. func New(ctx context.Context, config *models.CoreServiceConfig, log logger.Logger) (Service, error) { if config == nil { return nil, fmt.Errorf("%w: database configuration missing", ErrFailedOpenDB) } cnpgPool, err := newCNPGPool(ctx, config, log) if err != nil { return nil, err } if cnpgPool == nil { return nil, fmt.Errorf("%w: CNPG configuration not provided", ErrFailedOpenDB) } db := &DB{ pgPool: cnpgPool, executor: cnpgPool, // Default to pool logger: log, deviceUpdatesMu: &sync.Mutex{}, } ... (clipped 3 lines) ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary>Generic: Secure Error Handling</summary> **Objective:** To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. **Status:** <a href='https://github.com/carverauto/serviceradar/pull/2321/files#diff-c5919d757d10f4573450733578c3072c7a67203c2a205ac884c084431460268bR123-R143'>Error exposure unknown</a>: The diff adjusts TLS validation and connection URL building but does not show the call sites that surface these errors, so it cannot be confirmed whether detailed internal errors are only logged internally and not returned to end-users. <details open><summary>Referred Code</summary> ```go query.Set("sslmode", sslMode) if cfg.TLS != nil { caFile := resolveCNPGTLSPath(cfg, cfg.TLS.CAFile) // CA file is required for TLS verification (verify-ca, verify-full modes) if caFile == "" { return url.URL{}, ErrCNPGLackingTLSFiles } query.Set("sslrootcert", caFile) // Client cert and key are optional (only required if server demands client auth) certFile := resolveCNPGTLSPath(cfg, cfg.TLS.CertFile) keyFile := resolveCNPGTLSPath(cfg, cfg.TLS.KeyFile) if certFile != "" && keyFile != "" { query.Set("sslcert", certFile) query.Set("sslkey", keyFile) } } ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary>Generic: Secure Logging Practices</summary> **Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. **Status:** <a href='https://github.com/carverauto/serviceradar/pull/2321/files#diff-b150c895e524739f2e3d478e2384097fc7a7cf5abbcff278c14d82aa0a62b572R38-R44'>Logging format unknown</a>: The PR adds/updates operational log output and configuration, but the diff does not provide enough evidence about structured logging or whether runtime logs could include sensitive data. <details open><summary>Referred Code</summary> ```shell echo "⏳ Waiting for CNPG pods to become Ready..." kubectl wait --for=condition=Ready --timeout=600s pod -l "${SELECTOR}" -n "${NAMESPACE}" echo "🔁 Ash migrations are applied by core-elx on startup (SERVICERADAR_CORE_RUN_MIGRATIONS=true)." echo "♻️ Restarting core services so they reconnect to the fresh database..." for deployment in serviceradar-core serviceradar-datasvc \ ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary>Generic: Security-First Input Validation and Data Handling</summary> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities **Status:** <a href='https://github.com/carverauto/serviceradar/pull/2321/files#diff-c5919d757d10f4573450733578c3072c7a67203c2a205ac884c084431460268bR123-R145'>TLS mode handling</a>: Client certificate handling is made optional and CA enforcement is changed, but without the full context of <code>sslmode</code> derivation and config sources it requires verification that all TLS modes and external config inputs are validated and enforced as intended. <details open><summary>Referred Code</summary> ```go query.Set("sslmode", sslMode) if cfg.TLS != nil { caFile := resolveCNPGTLSPath(cfg, cfg.TLS.CAFile) // CA file is required for TLS verification (verify-ca, verify-full modes) if caFile == "" { return url.URL{}, ErrCNPGLackingTLSFiles } query.Set("sslrootcert", caFile) // Client cert and key are optional (only required if server demands client auth) certFile := resolveCNPGTLSPath(cfg, cfg.TLS.CertFile) keyFile := resolveCNPGTLSPath(cfg, cfg.TLS.KeyFile) if certFile != "" && keyFile != "" { query.Set("sslcert", certFile) query.Set("sslkey", keyFile) } } ... (clipped 2 lines) ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant 🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label </details>

qodo-code-review[bot] commented

2026-01-17 17:56:34 +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/2321#issuecomment-3764161377
Original created: 2026-01-17T17:56:34Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Nil-guard TLS normalization Add a `nil` check for the `tls` parameter at the beginning of the `normalizeTLSPaths` function to prevent a potential panic. pkg/config/config.go [568-571] `func (c Config) normalizeTLSPaths(tls models.TLSConfig, certDir string) { + if tls == nil { + return + } // Empty paths are left empty (not converted to directory paths). if tls.CertFile != "" && !filepath.IsAbs(tls.CertFile) { tls.CertFile = filepath.Join(certDir, tls.CertFile) }` Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a potential nil pointer dereference, which would cause a panic, and provides a simple guard to prevent the crash, significantly improving the code's robustness.	Medium
General	Add a timeout to the wait loop Add a timeout to the shell script's `while` loop to prevent it from running indefinitely if the certificate file is not generated. helm/serviceradar/templates/cnpg-client-ca-secret-job.yaml [27-41] - -c - \| - # Wait for cert-generator to complete + # Wait for cert-generator to complete with a timeout echo "Waiting for cnpg-ca.pem to exist..." + timeout=300 # 5 minutes + elapsed=0 while [ ! -f /certs/cnpg-ca.pem ]; do + if [ $elapsed -ge $timeout ]; then + echo "Error: Timed out waiting for /certs/cnpg-ca.pem" >&2 + exit 1 + fi sleep 2 + elapsed=$((elapsed + 2)) done echo "Found cnpg-ca.pem, creating secret..." # Create or update the secret with the CNPG client CA kubectl create secret generic serviceradar-cnpg-client-ca \ --from-file=ca.crt=/certs/cnpg-ca.pem \ --dry-run=client -o yaml \| kubectl apply -f - echo "Secret serviceradar-cnpg-client-ca created/updated" Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a potential infinite loop in the Kubernetes Job and proposes adding a timeout, which improves the script's robustness and makes failure detection faster and more explicit.	Low
Possible issue	Fix a race condition in process startup To prevent a race condition where multiple applications might start `ProcessRegistry` simultaneously, use a dedicated starter process to serialize the check and startup logic. elixir/serviceradar_core/lib/serviceradar/application.ex [198-211] defp registry_children do if Application.get_env(:serviceradar_core, :registries_enabled, true) do - # ProcessRegistry provides Horde registry + DynamicSupervisor as child_specs - process_registry_specs = - if Process.whereis(ServiceRadar.ProcessRegistry.registry_name()) \|\| - Process.whereis(ServiceRadar.ProcessRegistry.supervisor_name()) do - [] - else - ServiceRadar.ProcessRegistry.child_specs() - end + # The starter process will conditionally start the registry to avoid race conditions. + [ + ServiceRadar.ProcessRegistry.Starter, + # Gateway tracker (ETS-based) - process_registry_specs ++ - [ - # Gateway tracker (ETS-based) - Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential race condition in the process startup logic and proposes a more robust solution using a starter process, which would improve the system's reliability.	Medium
Possible issue	Restrict TLS setup to non-disabled SSL modes In `buildCNPGConnURL`, wrap the TLS configuration block in a condition to ensure it only runs when `sslMode` is not `disable`. pkg/db/cnpg_pool.go [125-143] -if cfg.TLS != nil { +if cfg.TLS != nil && sslMode != cnpgSSLModeDisable { caFile := resolveCNPGTLSPath(cfg, cfg.TLS.CAFile) // CA file is required for TLS verification (verify-ca, verify-full modes) if caFile == "" { return url.URL{}, ErrCNPGLackingTLSFiles } query.Set("sslrootcert", caFile) // Client cert and key are optional (only required if server demands client auth) certFile := resolveCNPGTLSPath(cfg, cfg.TLS.CertFile) keyFile := resolveCNPGTLSPath(cfg, cfg.TLS.KeyFile) if certFile != "" && keyFile != "" { query.Set("sslcert", certFile) query.Set("sslkey", keyFile) } } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that TLS file validation should not occur when SSL is disabled, and the proposed fix makes the connection logic more robust and flexible for different SSL modes.	Medium
Update

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2321#issuecomment-3764161377 Original created: 2026-01-17T17:56:34Z --- ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td>Category</td><td align=left>Suggestion                                                                                                                                    </td><td align=center>Impact</td></tr><tbody><tr><td rowspan=2>General</td> <td> <details><summary>Nil-guard TLS normalization</summary> ___ **Add a <code>nil</code> check for the <code>tls</code> parameter at the beginning of the <code>normalizeTLSPaths</code> function to prevent a potential panic.** [pkg/config/config.go [568-571]](https://github.com/carverauto/serviceradar/pull/2321/files#diff-a3d824da3c42420cd5cbb0a4a2c0e7b5bfddd819652788a0596d195dc6e31fa5R568-R571) ```diff func (c *Config) normalizeTLSPaths(tls *models.TLSConfig, certDir string) { + if tls == nil { + return + } // Empty paths are left empty (not converted to directory paths). if tls.CertFile != "" && !filepath.IsAbs(tls.CertFile) { tls.CertFile = filepath.Join(certDir, tls.CertFile) } ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: The suggestion correctly identifies a potential nil pointer dereference, which would cause a panic, and provides a simple guard to prevent the crash, significantly improving the code's robustness. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Add a timeout to the wait loop</summary> ___ **Add a timeout to the shell script's <code>while</code> loop to prevent it from running indefinitely if the certificate file is not generated.** [helm/serviceradar/templates/cnpg-client-ca-secret-job.yaml [27-41]](https://github.com/carverauto/serviceradar/pull/2321/files#diff-09fdf0d1b7a1d044a233516added81d55d6a50127e68504ac6e507d494ed12b6R27-R41) ```diff - -c - | - # Wait for cert-generator to complete + # Wait for cert-generator to complete with a timeout echo "Waiting for cnpg-ca.pem to exist..." + timeout=300 # 5 minutes + elapsed=0 while [ ! -f /certs/cnpg-ca.pem ]; do + if [ $elapsed -ge $timeout ]; then + echo "Error: Timed out waiting for /certs/cnpg-ca.pem" >&2 + exit 1 + fi sleep 2 + elapsed=$((elapsed + 2)) done echo "Found cnpg-ca.pem, creating secret..." # Create or update the secret with the CNPG client CA kubectl create secret generic serviceradar-cnpg-client-ca \ --from-file=ca.crt=/certs/cnpg-ca.pem \ --dry-run=client -o yaml | kubectl apply -f - echo "Secret serviceradar-cnpg-client-ca created/updated" ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: The suggestion correctly identifies a potential infinite loop in the Kubernetes Job and proposes adding a timeout, which improves the script's robustness and makes failure detection faster and more explicit. </details></details></td><td align=center>Low </td></tr><tr><td rowspan=2>Possible issue</td> <td> <details><summary>Fix a race condition in process startup</summary> ___ **To prevent a race condition where multiple applications might start <code>ProcessRegistry</code> simultaneously, use a dedicated starter process to serialize the check and startup logic.** [elixir/serviceradar_core/lib/serviceradar/application.ex [198-211]](https://github.com/carverauto/serviceradar/pull/2321/files#diff-a9ffbf400b7f9b22cd8980c41286c54fe373f1f1a8684bb6a344a5fb39b178d0R198-R211) ```diff defp registry_children do if Application.get_env(:serviceradar_core, :registries_enabled, true) do - # ProcessRegistry provides Horde registry + DynamicSupervisor as child_specs - process_registry_specs = - if Process.whereis(ServiceRadar.ProcessRegistry.registry_name()) || - Process.whereis(ServiceRadar.ProcessRegistry.supervisor_name()) do - [] - else - ServiceRadar.ProcessRegistry.child_specs() - end + # The starter process will conditionally start the registry to avoid race conditions. + [ + ServiceRadar.ProcessRegistry.Starter, + # Gateway tracker (ETS-based) - process_registry_specs ++ - [ - # Gateway tracker (ETS-based) - ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies a potential race condition in the process startup logic and proposes a more robust solution using a starter process, which would improve the system's reliability. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Restrict TLS setup to non-disabled SSL modes</summary> ___ **In <code>buildCNPGConnURL</code>, wrap the TLS configuration block in a condition to ensure it only runs when <code>sslMode</code> is not <code>disable</code>.** [pkg/db/cnpg_pool.go [125-143]](https://github.com/carverauto/serviceradar/pull/2321/files#diff-c5919d757d10f4573450733578c3072c7a67203c2a205ac884c084431460268bR125-R143) ```diff -if cfg.TLS != nil { +if cfg.TLS != nil && sslMode != cnpgSSLModeDisable { caFile := resolveCNPGTLSPath(cfg, cfg.TLS.CAFile) // CA file is required for TLS verification (verify-ca, verify-full modes) if caFile == "" { return url.URL{}, ErrCNPGLackingTLSFiles } query.Set("sslrootcert", caFile) // Client cert and key are optional (only required if server demands client auth) certFile := resolveCNPGTLSPath(cfg, cfg.TLS.CertFile) keyFile := resolveCNPGTLSPath(cfg, cfg.TLS.KeyFile) if certFile != "" && keyFile != "" { query.Set("sslcert", certFile) query.Set("sslkey", keyFile) } } ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly points out that TLS file validation should not occur when SSL is disabled, and the proposed fix makes the connection logic more robust and flexible for different SSL modes. </details></details></td><td align=center>Medium </td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td><td></td></tr></tbody></table>