carverauto/serviceradar

Fork 0

2102 bugdocker docker compose stack not seeding kv with configs #2542

Merged

mfreeman451 merged 3 commits from refs/pull/2542/head into main

2025-12-11 06:40:43 +00:00

mfreeman451 commented

2025-12-11 06:35:53 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2103
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2103
Original created: 2025-12-11T06:35:53Z
Original updated: 2025-12-11T06:40:43Z
Original head: carverauto/serviceradar:2102-bugdocker-docker-compose-stack-not-seeding-kv-with-configs
Original base: main
Original merged: 2025-12-11T06:40:43Z 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

Bug fix, Enhancement

Description

Enable KV-backed configuration in Docker Compose services to seed defaults into datasvc on first boot
Fix db-event-writer service to retry consumer connection instead of failing on startup
Prevent zen consumer from restarting on initial KV watch event (first sync)
Add NATS context setup script and debugging tools to serviceradar-tools image
Update all Compose services to use KV configuration source with proper mTLS credentials

Diagram Walkthrough

flowchart LR
  A["Docker Compose Services"] -->|CONFIG_SOURCE=kv| B["datasvc KV Store"]
  B -->|seed defaults| C["config/core.json<br/>config/agent.json<br/>config/poller.json"]
  D["db-event-writer"] -->|retry loop| E["NATS Consumer"]
  F["zen consumer"] -->|skip initial| G["KV Watch Events"]
  H["serviceradar-tools"] -->|includes| I["NATS context setup<br/>debugging aliases"]

File Walkthrough

Relevant files

Bug fix

2 files

service.go `Implement retry loop for NATS consumer connection`	+24/-20
main.rs `Skip initial KV watch event to prevent restart`	+6/-0

Enhancement

2 files

setup-nats-context.sh `Add NATS context configuration script for debugging`	+46/-0
Dockerfile.tools `Bundle NATS context setup and debugging tools`	+23/-8

Configuration changes

3 files

docker-compose.yml `Enable KV configuration source for all services`	+81/-28
docker-compose.podman.yml `Enable KV configuration source for podman variant`	+81/-28
poller-stack.compose.yml `Enable KV configuration for poller stack variant`	+8/-0

Documentation

5 files

tools-motd.txt `Update NATS context and certificate documentation`	+4/-4
docker-setup.md `Document KV seeding verification and watcher telemetry`	+17/-0
proposal.md `Define KV seeding fix proposal and requirements`	+15/-0
spec.md `Add Docker Compose KV bootstrap specification`	+9/-0
tasks.md `Document implementation tasks and checklist`	+7/-0

Imported from GitHub pull request. Original GitHub pull request: #2103 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2103 Original created: 2025-12-11T06:35:53Z Original updated: 2025-12-11T06:40:43Z Original head: carverauto/serviceradar:2102-bugdocker-docker-compose-stack-not-seeding-kv-with-configs Original base: main Original merged: 2025-12-11T06:40:43Z 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** Bug fix, Enhancement ___ ### **Description** - Enable KV-backed configuration in Docker Compose services to seed defaults into datasvc on first boot - Fix db-event-writer service to retry consumer connection instead of failing on startup - Prevent zen consumer from restarting on initial KV watch event (first sync) - Add NATS context setup script and debugging tools to serviceradar-tools image - Update all Compose services to use KV configuration source with proper mTLS credentials ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Docker Compose Services"] -->|CONFIG_SOURCE=kv| B["datasvc KV Store"] B -->|seed defaults| C["config/core.json<br/>config/agent.json<br/>config/poller.json"] D["db-event-writer"] -->|retry loop| E["NATS Consumer"] F["zen consumer"] -->|skip initial| G["KV Watch Events"] H["serviceradar-tools"] -->|includes| I["NATS context setup<br/>debugging aliases"] ``` <details><summary><h3>File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Bug fix</strong></td><td><details><summary>2 files</summary><table> <tr> <td><strong>service.go</strong><dd><code>Implement retry loop for NATS consumer connection</code>                </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2103/files#diff-9f9f48b11e7670c7ae374abc41327adf4617972b214f1c168e9da53d3cd7b609">+24/-20</a>  </td> </tr> <tr> <td><strong>main.rs</strong><dd><code>Skip initial KV watch event to prevent restart</code>                      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2103/files#diff-78494042ba6078c9704eeb6e374d97a3481a5a74426db8e95ed34da15f9133b2">+6/-0</a>      </td> </tr> </table></details></td></tr><tr><td><strong>Enhancement</strong></td><td><details><summary>2 files</summary><table> <tr> <td><strong>setup-nats-context.sh</strong><dd><code>Add NATS context configuration script for debugging</code>            </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2103/files#diff-201a8197495066d5f2189460b9c4a5fb49a423d11ec7c7a63ad575f12945b6a4">+46/-0</a>    </td> </tr> <tr> <td><strong>Dockerfile.tools</strong><dd><code>Bundle NATS context setup and debugging tools</code>                        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2103/files#diff-0258db71e4070e342198965f1d046f3097640850b037df8a2287a7e239630add">+23/-8</a>    </td> </tr> </table></details></td></tr><tr><td><strong>Configuration changes</strong></td><td><details><summary>3 files</summary><table> <tr> <td><strong>docker-compose.yml</strong><dd><code>Enable KV configuration source for all services</code>                    </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2103/files#diff-e45e45baeda1c1e73482975a664062aa56f20c03dd9d64a827aba57775bed0d3">+81/-28</a>  </td> </tr> <tr> <td><strong>docker-compose.podman.yml</strong><dd><code>Enable KV configuration source for podman variant</code>                </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2103/files#diff-77a7fc5fac1d23ddc900d0f5128aedde1fe22302ba66820a704a8e5d13a15dbb">+81/-28</a>  </td> </tr> <tr> <td><strong>poller-stack.compose.yml</strong><dd><code>Enable KV configuration for poller stack variant</code>                  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2103/files#diff-f3b5c991c2c1f7646db0ca4ed9bcb5df0f313ce6a05d8f3c890f80c873f776f5">+8/-0</a>      </td> </tr> </table></details></td></tr><tr><td><strong>Documentation</strong></td><td><details><summary>5 files</summary><table> <tr> <td><strong>tools-motd.txt</strong><dd><code>Update NATS context and certificate documentation</code>                </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2103/files#diff-b66b550fcd1772acb59e343e845225193c6b9601e78a29a1f124b7bcd9ca636a">+4/-4</a>      </td> </tr> <tr> <td><strong>docker-setup.md</strong><dd><code>Document KV seeding verification and watcher telemetry</code>      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2103/files#diff-8604269dffb3ce4133e48cab374ca8e97745d0efbdef67cad792aeb5945fe5ec">+17/-0</a>    </td> </tr> <tr> <td><strong>proposal.md</strong><dd><code>Define KV seeding fix proposal and requirements</code>                    </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2103/files#diff-101e287b4968d5f8b7ad517d4b4e297f1220e566b5afa440b4f175635255061e">+15/-0</a>    </td> </tr> <tr> <td><strong>spec.md</strong><dd><code>Add Docker Compose KV bootstrap specification</code>                        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2103/files#diff-b630673220a5759a8738dd243c6c56dcded486bc046bb2193b70c3ae57349c7e">+9/-0</a>      </td> </tr> <tr> <td><strong>tasks.md</strong><dd><code>Document implementation tasks and checklist</code>                            </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2103/files#diff-4eec00439bd8c35c743af0c8455b7797af00593c93c3016de2991b797453e8e5">+7/-0</a>      </td> </tr> </table></details></td></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2025-12-11 06:36:29 +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/2103#issuecomment-3640457919
Original created: 2025-12-11T06:36:29Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance

⚪

Insecure context overwrite

Description: The script auto-creates and selects a NATS CLI context using mTLS cert/key paths from
environment variables without validating permissions or prompting, which could
unintentionally overwrite an existing context and expose credentials usage if run in
shared environments.
setup-nats-context.sh [23-33]

Referred Code

  nats context save "${CONTEXT_NAME}" \
    --description "${CONTEXT_DESC}" \
    --server "${NATS_URL}" \
    --tlsca "${NATS_CA}" \
    --tlscert "${NATS_CERT}" \
    --tlskey "${NATS_KEY}" \
    --select \
    >/dev/null 2>&1 || true

  echo "NATS context '${CONTEXT_NAME}' ready for ${NATS_URL}"
else

Ticket Compliance

🟡

🎫 #2102

🟢	Ensure Compose stack uses KV-backed configuration for services so KV seeding occurs.
	Provide steps or tooling to verify that KV seeding happened in the Compose environment.
	Prevent unnecessary restarts caused by initial KV watch events in consumers that react to KV updates.
⚪	In Docker Compose, services must seed configuration defaults into the KV (datasvc) on first boot, matching Kubernetes behavior.

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: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

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: 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:
Logging Context: New retry loop logs connection failures but it is unclear if critical actions elsewhere
include user/action context; diff does not show audit fields like user IDs or outcomes for
critical operations.

Referred Code

	s.logger.Info().
		Str("stream_name", s.cfg.StreamName).
		Str("consumer_name", s.cfg.ConsumerName).
		Msg("Connected to NATS consumer")
	return consumer, nil
}

// Treat transient connection/setup errors as retryable so we don't exit the process.
if ctx.Err() != nil {
	return nil, err
}

s.logger.Warn().Err(err).Msg("Failed to (re)establish NATS consumer; retrying")
if !sleepWithContext(ctx, s.retryDelay) {

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:
Secrets In Logs: Several services add KV and database connection environment variables; while no sensitive
values are printed in the diff, ensure runtime logs do not emit cert paths, keys, or DB
passwords as some tools/aliases echo certificate metadata.

Referred Code

- CONFIG_SOURCE=kv
- CONFIG_PATH=/etc/serviceradar/templates/db-event-writer.json
- KV_ADDRESS=datasvc:50057
- KV_SEC_MODE=mtls
- KV_CERT_DIR=/etc/serviceradar/certs
- KV_CERT_FILE=/etc/serviceradar/certs/db-event-writer.pem
- KV_KEY_FILE=/etc/serviceradar/certs/db-event-writer-key.pem
- KV_CA_FILE=/etc/serviceradar/certs/root.pem
- KV_SERVER_NAME=datasvc.serviceradar
- LOG_LEVEL=${LOG_LEVEL:-info}
- CNPG_HOST=${CNPG_HOST:-cnpg}
- CNPG_PORT=${CNPG_PORT:-5432}
- CNPG_DATABASE=${CNPG_DATABASE:-serviceradar}
- CNPG_USERNAME=${CNPG_USERNAME:-serviceradar}
- CNPG_PASSWORD=${CNPG_PASSWORD:-serviceradar}
- CNPG_SSL_MODE=${CNPG_SSL_MODE:-verify-full}
- CNPG_CERT_DIR=/etc/serviceradar/certs

Learn more about managing compliance generic rules or creating your own custom rules

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/2103#issuecomment-3640457919 Original created: 2025-12-11T06:36:29Z --- ## PR Compliance Guide 🔍  Below is a summary of compliance checks for this PR:<br> <table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr> <tr><td rowspan=1>⚪</td> <td><details><summary><strong>Insecure context overwrite</strong></summary><br> <b>Description:</b> The script auto-creates and selects a NATS CLI context using mTLS cert/key paths from <br>environment variables without validating permissions or prompting, which could <br>unintentionally overwrite an existing context and expose credentials usage if run in <br>shared environments.<br> <strong><a href='https://github.com/carverauto/serviceradar/pull/2103/files#diff-201a8197495066d5f2189460b9c4a5fb49a423d11ec7c7a63ad575f12945b6a4R23-R33'>setup-nats-context.sh [23-33]</a></strong><br> <details open><summary>Referred Code</summary> ```shell nats context save "${CONTEXT_NAME}" \ --description "${CONTEXT_DESC}" \ --server "${NATS_URL}" \ --tlsca "${NATS_CA}" \ --tlscert "${NATS_CERT}" \ --tlskey "${NATS_KEY}" \ --select \ >/dev/null 2>&1 || true echo "NATS context '${CONTEXT_NAME}' ready for ${NATS_URL}" else ``` </details></details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>🟡</td> <td> <details> <summary>🎫 <a href=https://github.com/carverauto/serviceradar/issues/2102>#2102</a></summary> <table width='100%'><tbody> <tr><td rowspan=3>🟢</td> <td>Ensure Compose stack uses KV-backed configuration for services so KV seeding occurs.</td></tr> <tr><td>Provide steps or tooling to verify that KV seeding happened in the Compose environment.</td></tr> <tr><td>Prevent unnecessary restarts caused by initial KV watch events in consumers that react to <br>KV updates.</td></tr> <tr><td rowspan=1>⚪</td> <td>In Docker Compose, services must seed configuration defaults into the KV (datasvc) on <br>first boot, matching Kubernetes behavior.</td></tr> </tbody></table> </details> </td></tr> <tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></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'><strong>Custom Compliance</strong></td></tr> <tr><td rowspan=4>🟢</td><td> <details><summary><strong>Generic: Meaningful Naming and Self-Documenting Code</strong></summary><br> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code <br>self-documenting<br> **Status:** Passed<br> > 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><strong>Generic: Robust Error Handling and Edge Case Management</strong></summary><br> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful <br>degradation<br> **Status:** Passed<br> > 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><strong>Generic: Secure Error Handling</strong></summary><br> **Objective:** To prevent the leakage of sensitive system information through error messages while <br>providing sufficient detail for internal debugging.<br> **Status:** Passed<br> > 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><strong>Generic: Security-First Input Validation and Data Handling</strong></summary><br> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent <br>vulnerabilities<br> **Status:** Passed<br> > 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=2>⚪</td> <td><details> <summary><strong>Generic: Comprehensive Audit Trails</strong></summary><br> **Objective:** To create a detailed and reliable record of critical system actions for security analysis <br>and compliance.<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2103/files#diff-9f9f48b11e7670c7ae374abc41327adf4617972b214f1c168e9da53d3cd7b609R188-R201'><strong>Logging Context</strong></a>: New retry loop logs connection failures but it is unclear if critical actions elsewhere <br>include user/action context; diff does not show audit fields like user IDs or outcomes for <br>critical operations.<br> <details open><summary>Referred Code</summary> ```go s.logger.Info(). Str("stream_name", s.cfg.StreamName). Str("consumer_name", s.cfg.ConsumerName). Msg("Connected to NATS consumer") return consumer, nil } // Treat transient connection/setup errors as retryable so we don't exit the process. if ctx.Err() != nil { return nil, err } s.logger.Warn().Err(err).Msg("Failed to (re)establish NATS consumer; retrying") if !sleepWithContext(ctx, s.retryDelay) { ``` </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><strong>Generic: Secure Logging Practices</strong></summary><br> **Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive <br>information like PII, PHI, or cardholder data.<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2103/files#diff-e45e45baeda1c1e73482975a664062aa56f20c03dd9d64a827aba57775bed0d3R551-R567'><strong>Secrets In Logs</strong></a>: Several services add KV and database connection environment variables; while no sensitive <br>values are printed in the diff, ensure runtime logs do not emit cert paths, keys, or DB <br>passwords as some tools/aliases echo certificate metadata.<br> <details open><summary>Referred Code</summary> ```yaml - CONFIG_SOURCE=kv - CONFIG_PATH=/etc/serviceradar/templates/db-event-writer.json - KV_ADDRESS=datasvc:50057 - KV_SEC_MODE=mtls - KV_CERT_DIR=/etc/serviceradar/certs - KV_CERT_FILE=/etc/serviceradar/certs/db-event-writer.pem - KV_KEY_FILE=/etc/serviceradar/certs/db-event-writer-key.pem - KV_CA_FILE=/etc/serviceradar/certs/root.pem - KV_SERVER_NAME=datasvc.serviceradar - LOG_LEVEL=${LOG_LEVEL:-info} - CNPG_HOST=${CNPG_HOST:-cnpg} - CNPG_PORT=${CNPG_PORT:-5432} - CNPG_DATABASE=${CNPG_DATABASE:-serviceradar} - CNPG_USERNAME=${CNPG_USERNAME:-serviceradar} - CNPG_PASSWORD=${CNPG_PASSWORD:-serviceradar} - CNPG_SSL_MODE=${CNPG_SSL_MODE:-verify-full} - CNPG_CERT_DIR=/etc/serviceradar/certs ``` </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">   </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant<br> 🟡 - Partial Compliant<br> 🔴 - Not Compliant<br> ⚪ - Requires Further Human Verification<br> 🏷️ - Compliance label<br> </details>

qodo-code-review[bot] commented

2025-12-11 06:37:37 +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/2103#issuecomment-3640460388
Original created: 2025-12-11T06:37:37Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Remove duplicate environment variable definition Remove the redundant `CONFIG_SOURCE=file` environment variable for the `agent` service, as it overrides the intended `CONFIG_SOURCE=kv` setting. docker/compose/poller-stack.compose.yml [102-113] environment: - - CONFIG_SOURCE=kv + - CONFIG_SOURCE=kv - CONFIG_PATH=/etc/serviceradar/config/agent.json - - KV_ADDRESS=datasvc:50057 - - KV_SEC_MODE=mtls - - KV_CERT_DIR=/etc/serviceradar/certs - - KV_CERT_FILE=/etc/serviceradar/certs/agent.pem - - KV_KEY_FILE=/etc/serviceradar/certs/agent-key.pem - - KV_CA_FILE=/etc/serviceradar/certs/root.pem - - KV_SERVER_NAME=datasvc.serviceradar + - KV_ADDRESS=datasvc:50057 + - KV_SEC_MODE=mtls + - KV_CERT_DIR=/etc/serviceradar/certs + - KV_CERT_FILE=/etc/serviceradar/certs/agent.pem + - KV_KEY_FILE=/etc/serviceradar/certs/agent-key.pem + - KV_CA_FILE=/etc/serviceradar/certs/root.pem + - KV_SERVER_NAME=datasvc.serviceradar - LOG_LEVEL=${LOG_LEVEL:-info} - - CONFIG_SOURCE=file Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a duplicated `CONFIG_SOURCE` environment variable which would cause the service to ignore the intended `kv` configuration source, thus fixing a bug.	High
High-level	Use Docker Compose extension fields Refactor the Docker Compose files to eliminate duplicated KV configuration. Use YAML extension fields and anchors to define a common configuration block and merge it into each service. Examples: docker-compose.yml [264-272] `- CONFIG_SOURCE=kv - CONFIG_PATH=/etc/serviceradar/agent.json - KV_ADDRESS=datasvc:50057 - KV_SEC_MODE=mtls - KV_CERT_DIR=/etc/serviceradar/certs - KV_CERT_FILE=/etc/serviceradar/certs/agent.pem - KV_KEY_FILE=/etc/serviceradar/certs/agent-key.pem - KV_CA_FILE=/etc/serviceradar/certs/root.pem - KV_SERVER_NAME=datasvc.serviceradar` docker-compose.yml [385-396] `- CONFIG_SOURCE=kv - CONFIG_PATH=/etc/serviceradar/sync.json - CORE_ADDRESS=core:50052 - CORE_SEC_MODE=mtls - CORE_CERT_DIR=/etc/serviceradar/certs - KV_ADDRESS=datasvc:50057 - KV_SEC_MODE=mtls - KV_CERT_DIR=/etc/serviceradar/certs - KV_CERT_FILE=/etc/serviceradar/certs/sync.pem - KV_KEY_FILE=/etc/serviceradar/certs/sync-key.pem ... (clipped 2 lines)` Solution Walkthrough: Before: # In docker-compose.yml services: agent: environment: - CONFIG_SOURCE=kv - KV_ADDRESS=datasvc:50057 - KV_SEC_MODE=mtls - KV_CERT_DIR=/etc/serviceradar/certs - KV_CERT_FILE=/etc/serviceradar/certs/agent.pem - KV_KEY_FILE=/etc/serviceradar/certs/agent-key.pem - KV_CA_FILE=/etc/serviceradar/certs/root.pem - KV_SERVER_NAME=datasvc.serviceradar ... sync: environment: - CONFIG_SOURCE=kv - KV_ADDRESS=datasvc:50057 - KV_SEC_MODE=mtls - KV_CERT_DIR=/etc/serviceradar/certs - KV_CERT_FILE=/etc/serviceradar/certs/sync.pem - KV_KEY_FILE=/etc/serviceradar/certs/sync-key.pem - KV_CA_FILE=/etc/serviceradar/certs/root.pem - KV_SERVER_NAME=datasvc.serviceradar ... # ... and so on for other services After: # In docker-compose.yml x-kv-config: &kv-config CONFIG_SOURCE: kv KV_ADDRESS: datasvc:50057 KV_SEC_MODE: mtls KV_CERT_DIR: /etc/serviceradar/certs KV_CA_FILE: /etc/serviceradar/certs/root.pem KV_SERVER_NAME: datasvc.serviceradar services: agent: environment: <<: kv-config KV_CERT_FILE: /etc/serviceradar/certs/agent.pem KV_KEY_FILE: /etc/serviceradar/certs/agent-key.pem ... sync: environment: <<: kv-config KV_CERT_FILE: /etc/serviceradar/certs/sync.pem KV_KEY_FILE: /etc/serviceradar/certs/sync-key.pem ... # ... and so on for other services Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies significant duplication of KV configuration environment variables across multiple services in the compose files, and proposing YAML anchors to centralize this configuration would substantially improve maintainability.	Low
General	Update outdated image tag in documentation Update the `serviceradar-tools` image tag in the documentation from `v1.0.65` to `v1.0.67` to match the version used elsewhere in the PR. docs/docs/docker-setup.md [154-161] `docker run --rm --network serviceradar_serviceradar-net \ -v serviceradar_cert-data:/etc/serviceradar/certs \ - ghcr.io/carverauto/serviceradar-tools:${APP_TAG:-v1.0.65} \ + ghcr.io/carverauto/serviceradar-tools:${APP_TAG:-v1.0.67} \ nats --server tls://nats:4222 \ --tlsca /etc/serviceradar/certs/root.pem \ --tlscert /etc/serviceradar/certs/poller.pem \ --tlskey /etc/serviceradar/certs/poller-key.pem \ kv ls serviceradar-datasvc` Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies an outdated image tag in a documentation command, which improves the accuracy and usability of the setup instructions.	Low
More

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2103#issuecomment-3640460388 Original created: 2025-12-11T06:37:37Z --- ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion                                                                                                                                    </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>Possible issue</td> <td> <details><summary>Remove duplicate environment variable definition</summary> ___ **Remove the redundant <code>CONFIG_SOURCE=file</code> environment variable for the <code>agent</code> <br>service, as it overrides the intended <code>CONFIG_SOURCE=kv</code> setting.** [docker/compose/poller-stack.compose.yml [102-113]](https://github.com/carverauto/serviceradar/pull/2103/files#diff-f3b5c991c2c1f7646db0ca4ed9bcb5df0f313ce6a05d8f3c890f80c873f776f5R102-R113) ```diff environment: - - CONFIG_SOURCE=kv + - CONFIG_SOURCE=kv - CONFIG_PATH=/etc/serviceradar/config/agent.json - - KV_ADDRESS=datasvc:50057 - - KV_SEC_MODE=mtls - - KV_CERT_DIR=/etc/serviceradar/certs - - KV_CERT_FILE=/etc/serviceradar/certs/agent.pem - - KV_KEY_FILE=/etc/serviceradar/certs/agent-key.pem - - KV_CA_FILE=/etc/serviceradar/certs/root.pem - - KV_SERVER_NAME=datasvc.serviceradar + - KV_ADDRESS=datasvc:50057 + - KV_SEC_MODE=mtls + - KV_CERT_DIR=/etc/serviceradar/certs + - KV_CERT_FILE=/etc/serviceradar/certs/agent.pem + - KV_KEY_FILE=/etc/serviceradar/certs/agent-key.pem + - KV_CA_FILE=/etc/serviceradar/certs/root.pem + - KV_SERVER_NAME=datasvc.serviceradar - LOG_LEVEL=${LOG_LEVEL:-info} - - CONFIG_SOURCE=file ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: The suggestion correctly identifies a duplicated `CONFIG_SOURCE` environment variable which would cause the service to ignore the intended `kv` configuration source, thus fixing a bug. </details></details></td><td align=center>High </td></tr><tr><td rowspan=1>High-level</td> <td> <details><summary>Use Docker Compose extension fields</summary> ___ **Refactor the Docker Compose files to eliminate duplicated KV configuration. Use <br>YAML extension fields and anchors to define a common configuration block and <br>merge it into each service.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/2103/files#diff-e45e45baeda1c1e73482975a664062aa56f20c03dd9d64a827aba57775bed0d3R264-R272">docker-compose.yml [264-272]</a> </summary> ```yaml - CONFIG_SOURCE=kv - CONFIG_PATH=/etc/serviceradar/agent.json - KV_ADDRESS=datasvc:50057 - KV_SEC_MODE=mtls - KV_CERT_DIR=/etc/serviceradar/certs - KV_CERT_FILE=/etc/serviceradar/certs/agent.pem - KV_KEY_FILE=/etc/serviceradar/certs/agent-key.pem - KV_CA_FILE=/etc/serviceradar/certs/root.pem - KV_SERVER_NAME=datasvc.serviceradar ``` </details> <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/2103/files#diff-e45e45baeda1c1e73482975a664062aa56f20c03dd9d64a827aba57775bed0d3R385-R396">docker-compose.yml [385-396]</a> </summary> ```yaml - CONFIG_SOURCE=kv - CONFIG_PATH=/etc/serviceradar/sync.json - CORE_ADDRESS=core:50052 - CORE_SEC_MODE=mtls - CORE_CERT_DIR=/etc/serviceradar/certs - KV_ADDRESS=datasvc:50057 - KV_SEC_MODE=mtls - KV_CERT_DIR=/etc/serviceradar/certs - KV_CERT_FILE=/etc/serviceradar/certs/sync.pem - KV_KEY_FILE=/etc/serviceradar/certs/sync-key.pem ... (clipped 2 lines) ``` </details> ### Solution Walkthrough: #### Before: ```yaml # In docker-compose.yml services: agent: environment: - CONFIG_SOURCE=kv - KV_ADDRESS=datasvc:50057 - KV_SEC_MODE=mtls - KV_CERT_DIR=/etc/serviceradar/certs - KV_CERT_FILE=/etc/serviceradar/certs/agent.pem - KV_KEY_FILE=/etc/serviceradar/certs/agent-key.pem - KV_CA_FILE=/etc/serviceradar/certs/root.pem - KV_SERVER_NAME=datasvc.serviceradar ... sync: environment: - CONFIG_SOURCE=kv - KV_ADDRESS=datasvc:50057 - KV_SEC_MODE=mtls - KV_CERT_DIR=/etc/serviceradar/certs - KV_CERT_FILE=/etc/serviceradar/certs/sync.pem - KV_KEY_FILE=/etc/serviceradar/certs/sync-key.pem - KV_CA_FILE=/etc/serviceradar/certs/root.pem - KV_SERVER_NAME=datasvc.serviceradar ... # ... and so on for other services ``` #### After: ```yaml # In docker-compose.yml x-kv-config: &kv-config CONFIG_SOURCE: kv KV_ADDRESS: datasvc:50057 KV_SEC_MODE: mtls KV_CERT_DIR: /etc/serviceradar/certs KV_CA_FILE: /etc/serviceradar/certs/root.pem KV_SERVER_NAME: datasvc.serviceradar services: agent: environment: <<: *kv-config KV_CERT_FILE: /etc/serviceradar/certs/agent.pem KV_KEY_FILE: /etc/serviceradar/certs/agent-key.pem ... sync: environment: <<: *kv-config KV_CERT_FILE: /etc/serviceradar/certs/sync.pem KV_KEY_FILE: /etc/serviceradar/certs/sync-key.pem ... # ... and so on for other services ``` <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: The suggestion correctly identifies significant duplication of KV configuration environment variables across multiple services in the compose files, and proposing YAML anchors to centralize this configuration would substantially improve maintainability. </details></details></td><td align=center>Low </td></tr><tr><td rowspan=1>General</td> <td> <details><summary>Update outdated image tag in documentation</summary> ___ **Update the <code>serviceradar-tools</code> image tag in the documentation from <code>v1.0.65</code> to <br><code>v1.0.67</code> to match the version used elsewhere in the PR.** [docs/docs/docker-setup.md [154-161]](https://github.com/carverauto/serviceradar/pull/2103/files#diff-8604269dffb3ce4133e48cab374ca8e97745d0efbdef67cad792aeb5945fe5ecR154-R161) ```diff docker run --rm --network serviceradar_serviceradar-net \ -v serviceradar_cert-data:/etc/serviceradar/certs \ - ghcr.io/carverauto/serviceradar-tools:${APP_TAG:-v1.0.65} \ + ghcr.io/carverauto/serviceradar-tools:${APP_TAG:-v1.0.67} \ nats --server tls://nats:4222 \ --tlsca /etc/serviceradar/certs/root.pem \ --tlscert /etc/serviceradar/certs/poller.pem \ --tlskey /etc/serviceradar/certs/poller-key.pem \ kv ls serviceradar-datasvc ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: The suggestion correctly identifies an outdated image tag in a documentation command, which improves the accuracy and usability of the setup instructions. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] More  </td><td></td></tr></tbody></table>