helm chart password changes #2554

Merged

mfreeman451 merged 1 commit from refs/pull/2554/head into main

2025-12-13 03:16:41 +00:00

mfreeman451 commented

2025-12-13 03:08:29 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2115
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2115
Original created: 2025-12-13T03:08:29Z
Original updated: 2025-12-13T03:17:11Z
Original head: carverauto/serviceradar:chore/fix_e2e_staging_test
Original base: main
Original merged: 2025-12-13T03:16:41Z 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

Add pod restart trigger on database credentials changes via checksum annotation
Enable curl to follow redirects in e2e tests and health checks
Apply checksum annotation to core, db-event-writer, and srql deployments

Diagram Walkthrough

flowchart LR
  A["Database Credentials Secret"] -->|"lookup & checksum"| B["dbCredentialsChecksum Helper"]
  B -->|"annotation"| C["Pod Metadata"]
  C -->|"triggers restart"| D["Deployments Updated"]
  E["curl requests"] -->|"add -L flag"| F["Follow Redirects"]

File Walkthrough

Relevant files

Enhancement

_helpers.tpl `Add database credentials checksum helper template` helm/serviceradar/templates/_helpers.tpl Add new `dbCredentialsChecksum` template helper function Lookup existing database credentials secret and compute SHA256 checksum Fall back to random alphanumeric string if secret not found Enable automatic pod restart when database credentials change	+16/-0
core.yaml `Add checksum annotation to core deployment` helm/serviceradar/templates/core.yaml Add pod annotation with database credentials checksum Trigger pod restart when secret changes	+2/-0
db-event-writer.yaml `Add checksum annotation to db-event-writer deployment` helm/serviceradar/templates/db-event-writer.yaml Add pod annotation with database credentials checksum Ensure pod restarts when credentials are updated	+2/-0
srql.yaml `Add checksum annotation to srql deployment` helm/serviceradar/templates/srql.yaml Add pod annotation with database credentials checksum Trigger pod restart on credential changes	+2/-0

Bug fix

e2e-test.sh `Enable curl redirect following in e2e tests` scripts/e2e-test.sh Add `-L` flag to curl commands to follow HTTP redirects Apply to both POST and GET request paths Improves reliability of e2e test HTTP requests	+2/-2
e2e-tests.yml `Enable curl redirect following in health check` .github/workflows/e2e-tests.yml Add `-L` flag to curl command in health check Ensures proper handling of redirects during API readiness check	+1/-1

Imported from GitHub pull request. Original GitHub pull request: #2115 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2115 Original created: 2025-12-13T03:08:29Z Original updated: 2025-12-13T03:17:11Z Original head: carverauto/serviceradar:chore/fix_e2e_staging_test Original base: main Original merged: 2025-12-13T03:16:41Z 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** - Add pod restart trigger on database credentials changes via checksum annotation - Enable curl to follow redirects in e2e tests and health checks - Apply checksum annotation to core, db-event-writer, and srql deployments ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Database Credentials Secret"] -->|"lookup & checksum"| B["dbCredentialsChecksum Helper"] B -->|"annotation"| C["Pod Metadata"] C -->|"triggers restart"| D["Deployments Updated"] E["curl requests"] -->|"add -L flag"| F["Follow Redirects"] ``` <details><summary><h3>File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>_helpers.tpl</strong><dd><code>Add database credentials checksum helper template</code>                </dd></summary> <hr> helm/serviceradar/templates/_helpers.tpl <ul><li>Add new <code>dbCredentialsChecksum</code> template helper function<br> <li> Lookup existing database credentials secret and compute SHA256 <br>checksum<br> <li> Fall back to random alphanumeric string if secret not found<br> <li> Enable automatic pod restart when database credentials change</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2115/files#diff-3d59d815f528d134e097ce2c3e830c6eaa738e27b6645df1e9b18136cd5d3c0d">+16/-0</a>    </td> </tr> <tr> <td> <details> <summary><strong>core.yaml</strong><dd><code>Add checksum annotation to core deployment</code>                              </dd></summary> <hr> helm/serviceradar/templates/core.yaml <ul><li>Add pod annotation with database credentials checksum<br> <li> Trigger pod restart when secret changes</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2115/files#diff-06ab387d2c169d82a1de28b5e66c86f0417bd81b82a96246d0a2da8bfaa8d224">+2/-0</a>      </td> </tr> <tr> <td> <details> <summary><strong>db-event-writer.yaml</strong><dd><code>Add checksum annotation to db-event-writer deployment</code>        </dd></summary> <hr> helm/serviceradar/templates/db-event-writer.yaml <ul><li>Add pod annotation with database credentials checksum<br> <li> Ensure pod restarts when credentials are updated</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2115/files#diff-e4f899d11e5720f7049aa6fd632bd6993739410051bf65bc6fc8469739e5d2e4">+2/-0</a>      </td> </tr> <tr> <td> <details> <summary><strong>srql.yaml</strong><dd><code>Add checksum annotation to srql deployment</code>                              </dd></summary> <hr> helm/serviceradar/templates/srql.yaml <ul><li>Add pod annotation with database credentials checksum<br> <li> Trigger pod restart on credential changes</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2115/files#diff-263ca44548bae4940f960427ba7b95ed0e4a4fa24e342e4b480b0d4b6182f290">+2/-0</a>      </td> </tr> </table></td></tr><tr><td><strong>Bug fix</strong></td><td><table> <tr> <td> <details> <summary><strong>e2e-test.sh</strong><dd><code>Enable curl redirect following in e2e tests</code>                            </dd></summary> <hr> scripts/e2e-test.sh <ul><li>Add <code>-L</code> flag to curl commands to follow HTTP redirects<br> <li> Apply to both POST and GET request paths<br> <li> Improves reliability of e2e test HTTP requests</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2115/files#diff-dfa8a5c1766e059a8823270f22b8bf652b3a34894d9d940e1ac8bbe7ca2a806e">+2/-2</a>      </td> </tr> <tr> <td> <details> <summary><strong>e2e-tests.yml</strong><dd><code>Enable curl redirect following in health check</code>                      </dd></summary> <hr> .github/workflows/e2e-tests.yml <ul><li>Add <code>-L</code> flag to curl command in health check<br> <li> Ensures proper handling of redirects during API readiness check</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2115/files#diff-194218c48b9a0cdd03974145733804c2d992ca818529fe2fa69a501d8b5b1cc3">+1/-1</a>      </td> </tr> </table></td></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2025-12-13 03:08:57 +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/2115#issuecomment-3648825596
Original created: 2025-12-13T03:08:57Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
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: 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: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. 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: No audit scope: The changes add curl -L handling in test scripts and Helm annotations without introducing or affecting audit trails, which appears out of scope for this compliance item and cannot be assessed from the diff. Referred Code `response=$(curl -sL -w "\n%{http_code}" \ --max-time "$TIMEOUT" \ -X "$method" \ -H "Content-Type: application/json" \ ${auth_header:+-H "Authorization: Bearer $ACCESS_TOKEN"} \ -d "$data" \ "$url" 2>&1) \|\| true else response=$(curl -sL -w "\n%{http_code}" \ --max-time "$TIMEOUT" \` 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: Secret handling: The helper computes a checksum of the entire secret data (base64 values) and falls back to a random hash when absent; while not exposing values directly, validating that lookup access controls and absence fallback do not cause unnecessary restarts requires environment-specific verification. Referred Code {{/* Generate checksum for db credentials to trigger pod restart when secret changes. Uses lookup to get current secret value, falls back to random if not found. */}} {{- define "serviceradar.dbCredentialsChecksum" -}} {{- $ns := default .Release.Namespace .Values.spire.namespace -}} {{- $cnpg := default (dict) .Values.cnpg -}} {{- $secretName := default "serviceradar-db-credentials" $cnpg.credentialsSecret -}} {{- $existingSecret := (lookup "v1" "Secret" $ns $secretName) -}} {{- if and $existingSecret $existingSecret.data -}} {{- $existingSecret.data \| toJson \| sha256sum -}} {{- else -}} {{- randAlphaNum 32 \| sha256sum -}} {{- end -}} {{- end -}} 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/2115#issuecomment-3648825596 Original created: 2025-12-13T03:08:57Z --- ## 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>🟢</td><td><details><summary><strong>No security concerns identified</strong></summary> No security vulnerabilities detected by AI analysis. Human verification advised for critical code. </details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary>🎫 <strong>No ticket provided </strong></summary> - [ ] Create ticket/issue  </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: 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:** 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/2115/files#diff-dfa8a5c1766e059a8823270f22b8bf652b3a34894d9d940e1ac8bbe7ca2a806eR112-R121'><strong>No audit scope</strong></a>: The changes add curl -L handling in test scripts and Helm annotations without introducing <br>or affecting audit trails, which appears out of scope for this compliance item and cannot <br>be assessed from the diff.<br> <details open><summary>Referred Code</summary> ```shell response=$(curl -sL -w "\n%{http_code}" \ --max-time "$TIMEOUT" \ -X "$method" \ -H "Content-Type: application/json" \ ${auth_header:+-H "Authorization: Bearer $ACCESS_TOKEN"} \ -d "$data" \ "$url" 2>&1) || true else response=$(curl -sL -w "\n%{http_code}" \ --max-time "$TIMEOUT" \ ``` </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: 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:** <br><a href='https://github.com/carverauto/serviceradar/pull/2115/files#diff-3d59d815f528d134e097ce2c3e830c6eaa738e27b6645df1e9b18136cd5d3c0dR166-R180'><strong>Secret handling</strong></a>: The helper computes a checksum of the entire secret data (base64 values) and falls back to <br>a random hash when absent; while not exposing values directly, validating that lookup <br>access controls and absence fallback do not cause unnecessary restarts requires <br>environment-specific verification.<br> <details open><summary>Referred Code</summary> ```smarty {{/* Generate checksum for db credentials to trigger pod restart when secret changes. Uses lookup to get current secret value, falls back to random if not found. */}} {{- define "serviceradar.dbCredentialsChecksum" -}} {{- $ns := default .Release.Namespace .Values.spire.namespace -}} {{- $cnpg := default (dict) .Values.cnpg -}} {{- $secretName := default "serviceradar-db-credentials" $cnpg.credentialsSecret -}} {{- $existingSecret := (lookup "v1" "Secret" $ns $secretName) -}} {{- if and $existingSecret $existingSecret.data -}} {{- $existingSecret.data | toJson | sha256sum -}} {{- else -}} {{- randAlphaNum 32 | sha256sum -}} {{- end -}} {{- end -}} ``` </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-13 03:09:50 +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/2115#issuecomment-3648826389
Original created: 2025-12-13T03:09:50Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Avoid unnecessary pod restarts on upgrades To prevent unnecessary pod restarts on `helm upgrade` when the database secret is missing, replace the random checksum generation with a static one, such as the checksum of an empty string. helm/serviceradar/templates/_helpers.tpl [177-179] `{{- else -}} -{{- randAlphaNum 32 \| sha256sum -}} +{{- "" \| sha256sum -}} {{- end -}}` Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a flaw where using `randAlphaNum` would cause unnecessary pod restarts on every `helm upgrade` if the secret is missing, which undermines deployment stability. The proposed fix is the standard and correct pattern for this scenario.	Medium
More

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2115#issuecomment-3648826389 Original created: 2025-12-13T03:09:50Z --- ## 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>Avoid unnecessary pod restarts on upgrades</summary> ___ **To prevent unnecessary pod restarts on <code>helm upgrade</code> when the database secret is <br>missing, replace the random checksum generation with a static one, such as the <br>checksum of an empty string.** [helm/serviceradar/templates/_helpers.tpl [177-179]](https://github.com/carverauto/serviceradar/pull/2115/files#diff-3d59d815f528d134e097ce2c3e830c6eaa738e27b6645df1e9b18136cd5d3c0dR177-R179) ```diff {{- else -}} -{{- randAlphaNum 32 | sha256sum -}} +{{- "" | sha256sum -}} {{- end -}} ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: The suggestion correctly identifies a flaw where using `randAlphaNum` would cause unnecessary pod restarts on every `helm upgrade` if the secret is missing, which undermines deployment stability. The proposed fix is the standard and correct pattern for this scenario. </details></details></td><td align=center>Medium </td></tr> <tr><td align="center" colspan="2"> - [ ] More  </td><td></td></tr></tbody></table>