carverauto/serviceradar

Fork 0

fixing jwks issues #2279

Merged

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

2025-10-05 18:24:05 +00:00

mfreeman451 commented

2025-10-05 18:23:16 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #1707
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1707
Original created: 2025-10-05T18:23:16Z
Original updated: 2025-10-05T18:24:33Z
Original head: carverauto/serviceradar:k8s/kong_config_jwks
Original base: main
Original merged: 2025-10-05T18:24:05Z by @mfreeman451

PR Type

Bug fix, Enhancement

Description

Fix JWKS key generation using dedicated init container
Separate template and runtime configuration paths
Improve configuration file handling and error reporting
Remove CLI dependency from main init script

Diagram Walkthrough

flowchart LR
  A["Template Config"] --> B["Init Container"]
  B --> C["Generate JWKS Keys"]
  C --> D["Runtime Config"]
  D --> E["Core Container"]

File Walkthrough

Relevant files

Bug fix

configmap.yaml

Refactor configuration handling and remove CLI dependency

k8s/demo/base/configmap.yaml

Separate template and runtime configuration paths
Remove serviceradar-cli dependency from init script
Improve configuration file handling with proper directory creation
Add warning for missing RS256 key material

+15/-15

Enhancement

serviceradar-core.yaml

Add init container for JWKS key generation

k8s/demo/base/serviceradar-core.yaml

Add dedicated init container for JWKS key generation
Use kong-config image with serviceradar-cli for key generation
Update CONFIG_PATH to use runtime configuration location
Add core-data volume mount for persistent configuration

+19/-1

Imported from GitHub pull request. Original GitHub pull request: #1707 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/1707 Original created: 2025-10-05T18:23:16Z Original updated: 2025-10-05T18:24:33Z Original head: carverauto/serviceradar:k8s/kong_config_jwks Original base: main Original merged: 2025-10-05T18:24:05Z by @mfreeman451 --- ### **PR Type** Bug fix, Enhancement ___ ### **Description** - Fix JWKS key generation using dedicated init container - Separate template and runtime configuration paths - Improve configuration file handling and error reporting - Remove CLI dependency from main init script ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Template Config"] --> B["Init Container"] B --> C["Generate JWKS Keys"] C --> D["Runtime Config"] D --> E["Core Container"] ``` <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><table> <tr> <td> <details> <summary><strong>configmap.yaml</strong><dd><code>Refactor configuration handling and remove CLI dependency</code></dd></summary> <hr> k8s/demo/base/configmap.yaml <ul><li>Separate template and runtime configuration paths<br> <li> Remove serviceradar-cli dependency from init script<br> <li> Improve configuration file handling with proper directory creation<br> <li> Add warning for missing RS256 key material</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1707/files#diff-f4548beaa0a3a01a46971c82c5647a0f3f49eb38d66dd939d06d19018173fcd6">+15/-15</a>  </td> </tr> </table></td></tr><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>serviceradar-core.yaml</strong><dd><code>Add init container for JWKS key generation</code>                              </dd></summary> <hr> k8s/demo/base/serviceradar-core.yaml <ul><li>Add dedicated init container for JWKS key generation<br> <li> Use kong-config image with serviceradar-cli for key generation<br> <li> Update CONFIG_PATH to use runtime configuration location<br> <li> Add core-data volume mount for persistent configuration</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1707/files#diff-2f484d8fe3bae65aace437568f6dd660c92f57b452f7bd1608083a8fe3716ba3">+19/-1</a>    </td> </tr> </table></td></tr></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2025-10-05 18:23: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/1707#issuecomment-3369235648
Original created: 2025-10-05T18:23:37Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Key rotation risk Description: The init container runs 'serviceradar-cli generate-jwt-keys --force' which may overwrite existing key material each deployment, potentially invalidating existing RS256 tokens and causing service disruption. serviceradar-core.yaml [21-37] Referred Code `- name: init-core-jwks image: ghcr.io/carverauto/serviceradar-kong-config:latest imagePullPolicy: Always command: - /bin/sh - -c - \| set -e mkdir -p /var/lib/serviceradar cp /etc/serviceradar/core.json /var/lib/serviceradar/core.json serviceradar-cli generate-jwt-keys --file /var/lib/serviceradar/core.json --bits 2048 --force volumeMounts: - name: serviceradar-config mountPath: /etc/serviceradar readOnly: true - name: core-data mountPath: /var/lib/serviceradar`
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
⚪	No custom compliance provided Follow the guide to enable custom compliance check.

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/1707#issuecomment-3369235648 Original created: 2025-10-05T18:23:37Z --- ## 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>Key rotation risk </strong></summary><br> <b>Description:</b> The init container runs 'serviceradar-cli generate-jwt-keys --force' which may overwrite <br>existing key material each deployment, potentially invalidating existing RS256 tokens and <br>causing service disruption.<br> <strong><a href='https://github.com/carverauto/serviceradar/pull/1707/files#diff-2f484d8fe3bae65aace437568f6dd660c92f57b452f7bd1608083a8fe3716ba3R21-R37'>serviceradar-core.yaml [21-37]</a></strong><br> <details open><summary>Referred Code</summary> ```yaml - name: init-core-jwks image: ghcr.io/carverauto/serviceradar-kong-config:latest imagePullPolicy: Always command: - /bin/sh - -c - | set -e mkdir -p /var/lib/serviceradar cp /etc/serviceradar/core.json /var/lib/serviceradar/core.json serviceradar-cli generate-jwt-keys --file /var/lib/serviceradar/core.json --bits 2048 --force volumeMounts: - name: serviceradar-config mountPath: /etc/serviceradar readOnly: true - name: core-data mountPath: /var/lib/serviceradar ``` </details></details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary>🎫 <strong>No ticket provided </summary></strong> - [ ] 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>⚪</td><td><details><summary><strong>No custom compliance provided</strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/'>guide</a> to enable custom compliance check. </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-10-05 18:24:33 +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/1707#issuecomment-3369236148
Original created: 2025-10-05T18:24:33Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category Suggestion Impact

Possible issue

Category	Suggestion	Impact
Possible issue	Prevent overwriting existing configuration files In the `init-core-jwks` init container, modify the script to check if the configuration file exists before copying the template to prevent overwriting generated JWT keys on pod restarts. k8s/demo/base/serviceradar-core.yaml [21-31] - name: init-core-jwks image: ghcr.io/carverauto/serviceradar-kong-config:latest imagePullPolicy: Always command: - /bin/sh - -c - \| set -e - mkdir -p /var/lib/serviceradar - cp /etc/serviceradar/core.json /var/lib/serviceradar/core.json - serviceradar-cli generate-jwt-keys --file /var/lib/serviceradar/core.json --bits 2048 --force + CONFIG_PATH="/var/lib/serviceradar/core.json" + CONFIG_DIR=$(dirname "$CONFIG_PATH") + mkdir -p "$CONFIG_DIR" + if [ ! -f "$CONFIG_PATH" ]; then + echo "Seeding runtime config for JWKS generation." + cp /etc/serviceradar/core.json "$CONFIG_PATH" + fi + serviceradar-cli generate-jwt-keys --file "$CONFIG_PATH" --bits 2048 --force Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical flaw where the init container would overwrite JWT keys on every pod restart, defeating the purpose of the persistent volume and potentially causing authentication issues for clients.	High
More

Prevent overwriting existing configuration files

In the init-core-jwks init container, modify the script to check if the
configuration file exists before copying the template to prevent overwriting
generated JWT keys on pod restarts.

k8s/demo/base/serviceradar-core.yaml [21-31]

 - name: init-core-jwks
   image: ghcr.io/carverauto/serviceradar-kong-config:latest
   imagePullPolicy: Always
   command:
   - /bin/sh
   - -c
   - |
     set -e
-    mkdir -p /var/lib/serviceradar
-    cp /etc/serviceradar/core.json /var/lib/serviceradar/core.json
-    serviceradar-cli generate-jwt-keys --file /var/lib/serviceradar/core.json --bits 2048 --force
+    CONFIG_PATH="/var/lib/serviceradar/core.json"
+    CONFIG_DIR=$(dirname "$CONFIG_PATH")
+    mkdir -p "$CONFIG_DIR"
+    if [ ! -f "$CONFIG_PATH" ]; then
+      echo "Seeding runtime config for JWKS generation."
+      cp /etc/serviceradar/core.json "$CONFIG_PATH"
+    fi
+    serviceradar-cli generate-jwt-keys --file "$CONFIG_PATH" --bits 2048 --force

Apply / Chat

Suggestion importance[1-10]: 9

Why: The suggestion correctly identifies a critical flaw where the init container would overwrite JWT keys on every pod restart, defeating the purpose of the persistent volume and potentially causing authentication issues for clients.

High

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1707#issuecomment-3369236148 Original created: 2025-10-05T18:24:33Z --- ## 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>Prevent overwriting existing configuration files</summary> ___ **In the <code>init-core-jwks</code> init container, modify the script to check if the <br>configuration file exists before copying the template to prevent overwriting <br>generated JWT keys on pod restarts.** [k8s/demo/base/serviceradar-core.yaml [21-31]](https://github.com/carverauto/serviceradar/pull/1707/files#diff-2f484d8fe3bae65aace437568f6dd660c92f57b452f7bd1608083a8fe3716ba3R21-R31) ```diff - name: init-core-jwks image: ghcr.io/carverauto/serviceradar-kong-config:latest imagePullPolicy: Always command: - /bin/sh - -c - | set -e - mkdir -p /var/lib/serviceradar - cp /etc/serviceradar/core.json /var/lib/serviceradar/core.json - serviceradar-cli generate-jwt-keys --file /var/lib/serviceradar/core.json --bits 2048 --force + CONFIG_PATH="/var/lib/serviceradar/core.json" + CONFIG_DIR=$(dirname "$CONFIG_PATH") + mkdir -p "$CONFIG_DIR" + if [ ! -f "$CONFIG_PATH" ]; then + echo "Seeding runtime config for JWKS generation." + cp /etc/serviceradar/core.json "$CONFIG_PATH" + fi + serviceradar-cli generate-jwt-keys --file "$CONFIG_PATH" --bits 2048 --force ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: The suggestion correctly identifies a critical flaw where the init container would overwrite JWT keys on every pod restart, defeating the purpose of the persistent volume and potentially causing authentication issues for clients. </details></details></td><td align=center>High </td></tr> <tr><td align="center" colspan="2"> - [ ] More  </td><td></td></tr></tbody></table>