adding service watcher #2288

Merged

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

2025-10-06 00:42:56 +00:00

mfreeman451 commented

2025-10-06 00:42:14 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #1717
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1717
Original created: 2025-10-06T00:42:14Z
Original updated: 2025-10-06T00:43:34Z
Original head: carverauto/serviceradar:k8s/auth_kong_jwks_issue
Original base: main
Original merged: 2025-10-06T00:42:56Z by @mfreeman451

PR Type

Enhancement

Description

Add Kong configuration watcher sidecar container
Implement dynamic Kong config updates via serviceradar-cli
Configure JWKS authentication and service routing
Add resource limits and health checks

Diagram Walkthrough

flowchart LR
  A["Kong Config Watcher"] --> B["serviceradar-cli render-kong"]
  B --> C["Generate Kong Config"]
  C --> D["Apply to Kong Admin API"]
  D --> E["Kong Gateway Updated"]

File Walkthrough

Relevant files

Enhancement

serviceradar-kong.yaml

Add Kong configuration watcher sidecar

k8s/demo/base/serviceradar-kong.yaml

Add kong-config-watcher sidecar container to Kong deployment
Configure dynamic Kong configuration rendering with JWKS
authentication
Set up continuous monitoring and config updates every 60 seconds
Add resource requests and limits for the watcher container

+47/-0

Imported from GitHub pull request. Original GitHub pull request: #1717 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/1717 Original created: 2025-10-06T00:42:14Z Original updated: 2025-10-06T00:43:34Z Original head: carverauto/serviceradar:k8s/auth_kong_jwks_issue Original base: main Original merged: 2025-10-06T00:42:56Z by @mfreeman451 --- ### **PR Type** Enhancement ___ ### **Description** - Add Kong configuration watcher sidecar container - Implement dynamic Kong config updates via serviceradar-cli - Configure JWKS authentication and service routing - Add resource limits and health checks ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Kong Config Watcher"] --> B["serviceradar-cli render-kong"] B --> C["Generate Kong Config"] C --> D["Apply to Kong Admin API"] D --> E["Kong Gateway Updated"] ``` <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>serviceradar-kong.yaml</strong><dd><code>Add Kong configuration watcher sidecar</code>                                      </dd></summary> <hr> k8s/demo/base/serviceradar-kong.yaml <ul><li>Add <code>kong-config-watcher</code> sidecar container to Kong deployment<br> <li> Configure dynamic Kong configuration rendering with JWKS <br>authentication<br> <li> Set up continuous monitoring and config updates every 60 seconds<br> <li> Add resource requests and limits for the watcher container</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1717/files#diff-e1426b570f5caae8eee31f02984392f1bc68d0c329ae2f1118de3272d654856e">+47/-0</a>    </td> </tr> </table></td></tr></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2025-10-06 00:42:32 +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/1717#issuecomment-3369582335
Original created: 2025-10-06T00:42:32Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Insecure admin API access Description: The sidecar executes a shell loop with 'curl' to Kong Admin API on 127.0.0.1:8001 without TLS or authentication, exposing the admin interface to unauthenticated local access if the admin API is not otherwise restricted and risking config tampering if the container is compromised. serviceradar-kong.yaml [97-126] Referred Code set -euo pipefail render() { /usr/local/bin/serviceradar-cli render-kong \ --jwks http://serviceradar-core:8090/auth/jwks.json \ --service http://serviceradar-core:8090 \ --path /api \ --srql-service http://serviceradar-srql:8080 \ --srql-path /api/query \ --out "$1" } wait-for-port --host serviceradar-core --port 8090 --attempts 60 --interval 5s --quiet wait-for-port --host serviceradar-srql --port 8080 --attempts 60 --interval 5s --quiet while true; do tmp_file=$(mktemp /tmp/kong-config.XXXXXX) if render "$tmp_file"; then if [ ! -f /opt/kong/kong.yml ] \|\| ! cmp -s "$tmp_file" /opt/kong/kong.yml; then mv "$tmp_file" /opt/kong/kong.yml http_code=$(curl -s -o /tmp/kong-config-apply.log -w "%{http_code}" -X POST http://127.0.0.1:8001/config -F "config=@/opt/kong/kong.yml") \|\| true ... (clipped 9 lines)
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/1717#issuecomment-3369582335 Original created: 2025-10-06T00:42:32Z --- ## 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 admin API access </strong></summary><br> <b>Description:</b> The sidecar executes a shell loop with 'curl' to Kong Admin API on 127.0.0.1:8001 without <br>TLS or authentication, exposing the admin interface to unauthenticated local access if the <br>admin API is not otherwise restricted and risking config tampering if the container is <br>compromised.<br> <strong><a href='https://github.com/carverauto/serviceradar/pull/1717/files#diff-e1426b570f5caae8eee31f02984392f1bc68d0c329ae2f1118de3272d654856eR97-R126'>serviceradar-kong.yaml [97-126]</a></strong><br> <details open><summary>Referred Code</summary> ```yaml set -euo pipefail render() { /usr/local/bin/serviceradar-cli render-kong \ --jwks http://serviceradar-core:8090/auth/jwks.json \ --service http://serviceradar-core:8090 \ --path /api \ --srql-service http://serviceradar-srql:8080 \ --srql-path /api/query \ --out "$1" } wait-for-port --host serviceradar-core --port 8090 --attempts 60 --interval 5s --quiet wait-for-port --host serviceradar-srql --port 8080 --attempts 60 --interval 5s --quiet while true; do tmp_file=$(mktemp /tmp/kong-config.XXXXXX) if render "$tmp_file"; then if [ ! -f /opt/kong/kong.yml ] || ! cmp -s "$tmp_file" /opt/kong/kong.yml; then mv "$tmp_file" /opt/kong/kong.yml http_code=$(curl -s -o /tmp/kong-config-apply.log -w "%{http_code}" -X POST http://127.0.0.1:8001/config -F "config=@/opt/kong/kong.yml") || true ... (clipped 9 lines) ``` </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-06 00:43: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/1717#issuecomment-3369583567
Original created: 2025-10-06T00:43:34Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Consider using a Kubernetes Operator It is suggested to replace the current polling-based sidecar script with a dedicated Kubernetes Operator. This change would create a more robust, event-driven, and idiomatic solution for managing Kong's configuration. Examples: k8s/demo/base/serviceradar-kong.yaml [90-136] `- name: kong-config-watcher image: ghcr.io/carverauto/serviceradar-kong-config:latest imagePullPolicy: Always command: - /bin/sh - -c - \| set -euo pipefail render() { ... (clipped 37 lines)` Solution Walkthrough: Before: `# k8s/demo/base/serviceradar-kong.yaml containers: - name: kong-config-watcher command: - /bin/sh - -c - \| while true; do render_config_to_tmp_file() if config_changed; then apply_config_via_curl() fi sleep 60 done` After: `# The sidecar is removed from serviceradar-kong.yaml. # An Operator and a Custom Resource would manage the configuration. # operator-deployment.yaml (new) apiVersion: apps/v1 kind: Deployment metadata: name: kong-config-operator # ... Operator logic runs in this pod # kong-config-cr.yaml (new) apiVersion: serviceradar.io/v1alpha1 kind: KongConfig metadata: name: my-kong-instance spec: # Operator watches this resource and updates Kong # when related services change.` Suggestion importance[1-10]: 8 __ Why: This is a significant architectural suggestion that correctly identifies the current polling sidecar's limitations and proposes a more robust, scalable, and idiomatic Kubernetes-native solution.	Medium
Possible issue	Ensure atomic configuration updates To ensure configuration consistency, first apply the new configuration from the temporary file to Kong, and only move it to `/opt/kong/kong.yml` if the application is successful. k8s/demo/base/serviceradar-kong.yaml [114-124] if render "$tmp_file"; then if [ ! -f /opt/kong/kong.yml ] \|\| ! cmp -s "$tmp_file" /opt/kong/kong.yml; then - mv "$tmp_file" /opt/kong/kong.yml - http_code=$(curl -s -o /tmp/kong-config-apply.log -w "%{http_code}" -X POST http://127.0.0.1:8001/config -F "config=@/opt/kong/kong.yml") \|\| true - echo "Applied Kong config update (HTTP ${http_code:-000})" >&2 + http_code=$(curl -s -o /tmp/kong-config-apply.log -w "%{http_code}" -X POST http://127.0.0.1:8001/config -F "config=@$tmp_file") \|\| true + if [ "${http_code:-0}" -ge 200 ] && [ "${http_code:-0}" -lt 300 ]; then + mv "$tmp_file" /opt/kong/kong.yml + echo "Applied Kong config update (HTTP ${http_code})" >&2 + else + echo "Failed to apply Kong config update (HTTP ${http_code:-000}), will retry." >&2 + rm "$tmp_file" + fi else rm "$tmp_file" fi else rm -f "$tmp_file" fi Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a logic flaw where the on-disk configuration file is updated before a successful application to Kong, potentially leaving the system in an inconsistent state. The proposed change makes the update process more atomic and robust.	Medium
General	Ensure temporary file cleanup on exit Add a `trap` command to ensure the temporary file created by `mktemp` is cleaned up when the script exits unexpectedly. k8s/demo/base/serviceradar-kong.yaml [113-124] tmp_file=$(mktemp /tmp/kong-config.XXXXXX) +trap 'rm -f "$tmp_file"' EXIT + if render "$tmp_file"; then if [ ! -f /opt/kong/kong.yml ] \|\| ! cmp -s "$tmp_file" /opt/kong/kong.yml; then mv "$tmp_file" /opt/kong/kong.yml + # The trap will clean up the moved file's old path, which is fine. + # We need a new trap to clean up the new path if we exit. + trap 'rm -f /opt/kong/kong.yml' EXIT http_code=$(curl -s -o /tmp/kong-config-apply.log -w "%{http_code}" -X POST http://127.0.0.1:8001/config -F "config=@/opt/kong/kong.yml") \|\| true echo "Applied Kong config update (HTTP ${http_code:-000})" >&2 - else - rm "$tmp_file" + # Unset trap if we are not exiting + trap - EXIT fi -else - rm -f "$tmp_file" fi +# The file is either moved or removed, so no explicit rm is needed here. +# The trap handles unexpected exits. Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly points out a missing cleanup mechanism for temporary files upon script termination, which is a good shell scripting practice. While this improves robustness, the impact is minor as the container's temporary filesystem is ephemeral.	Low
More

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1717#issuecomment-3369583567 Original created: 2025-10-06T00:43:34Z --- ## 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>High-level</td> <td> <details><summary>Consider using a Kubernetes Operator</summary> ___ **It is suggested to replace the current polling-based sidecar script with a <br>dedicated Kubernetes Operator. This change would create a more robust, <br>event-driven, and idiomatic solution for managing Kong's configuration.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/1717/files#diff-e1426b570f5caae8eee31f02984392f1bc68d0c329ae2f1118de3272d654856eR90-R136">k8s/demo/base/serviceradar-kong.yaml [90-136]</a> </summary> ```yaml - name: kong-config-watcher image: ghcr.io/carverauto/serviceradar-kong-config:latest imagePullPolicy: Always command: - /bin/sh - -c - | set -euo pipefail render() { ... (clipped 37 lines) ``` </details> ### Solution Walkthrough: #### Before: ```yaml # k8s/demo/base/serviceradar-kong.yaml containers: - name: kong-config-watcher command: - /bin/sh - -c - | while true; do render_config_to_tmp_file() if config_changed; then apply_config_via_curl() fi sleep 60 done ``` #### After: ```yaml # The sidecar is removed from serviceradar-kong.yaml. # An Operator and a Custom Resource would manage the configuration. # operator-deployment.yaml (new) apiVersion: apps/v1 kind: Deployment metadata: name: kong-config-operator # ... Operator logic runs in this pod # kong-config-cr.yaml (new) apiVersion: serviceradar.io/v1alpha1 kind: KongConfig metadata: name: my-kong-instance spec: # Operator watches this resource and updates Kong # when related services change. ``` <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: This is a significant architectural suggestion that correctly identifies the current polling sidecar's limitations and proposes a more robust, scalable, and idiomatic Kubernetes-native solution. </details></details></td><td align=center>Medium </td></tr><tr><td rowspan=1>Possible issue</td> <td> <details><summary>Ensure atomic configuration updates</summary> ___ **To ensure configuration consistency, first apply the new configuration from the <br>temporary file to Kong, and only move it to <code>/opt/kong/kong.yml</code> if the <br>application is successful.** [k8s/demo/base/serviceradar-kong.yaml [114-124]](https://github.com/carverauto/serviceradar/pull/1717/files#diff-e1426b570f5caae8eee31f02984392f1bc68d0c329ae2f1118de3272d654856eR114-R124) ```diff if render "$tmp_file"; then if [ ! -f /opt/kong/kong.yml ] || ! cmp -s "$tmp_file" /opt/kong/kong.yml; then - mv "$tmp_file" /opt/kong/kong.yml - http_code=$(curl -s -o /tmp/kong-config-apply.log -w "%{http_code}" -X POST http://127.0.0.1:8001/config -F "config=@/opt/kong/kong.yml") || true - echo "Applied Kong config update (HTTP ${http_code:-000})" >&2 + http_code=$(curl -s -o /tmp/kong-config-apply.log -w "%{http_code}" -X POST http://127.0.0.1:8001/config -F "config=@$tmp_file") || true + if [ "${http_code:-0}" -ge 200 ] && [ "${http_code:-0}" -lt 300 ]; then + mv "$tmp_file" /opt/kong/kong.yml + echo "Applied Kong config update (HTTP ${http_code})" >&2 + else + echo "Failed to apply Kong config update (HTTP ${http_code:-000}), will retry." >&2 + rm "$tmp_file" + fi else rm "$tmp_file" fi else rm -f "$tmp_file" fi ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: The suggestion correctly identifies a logic flaw where the on-disk configuration file is updated before a successful application to Kong, potentially leaving the system in an inconsistent state. The proposed change makes the update process more atomic and robust. </details></details></td><td align=center>Medium </td></tr><tr><td rowspan=1>General</td> <td> <details><summary>Ensure temporary file cleanup on exit</summary> ___ **Add a <code>trap</code> command to ensure the temporary file created by <code>mktemp</code> is cleaned up <br>when the script exits unexpectedly.** [k8s/demo/base/serviceradar-kong.yaml [113-124]](https://github.com/carverauto/serviceradar/pull/1717/files#diff-e1426b570f5caae8eee31f02984392f1bc68d0c329ae2f1118de3272d654856eR113-R124) ```diff tmp_file=$(mktemp /tmp/kong-config.XXXXXX) +trap 'rm -f "$tmp_file"' EXIT + if render "$tmp_file"; then if [ ! -f /opt/kong/kong.yml ] || ! cmp -s "$tmp_file" /opt/kong/kong.yml; then mv "$tmp_file" /opt/kong/kong.yml + # The trap will clean up the moved file's old path, which is fine. + # We need a new trap to clean up the new path if we exit. + trap 'rm -f /opt/kong/kong.yml' EXIT http_code=$(curl -s -o /tmp/kong-config-apply.log -w "%{http_code}" -X POST http://127.0.0.1:8001/config -F "config=@/opt/kong/kong.yml") || true echo "Applied Kong config update (HTTP ${http_code:-000})" >&2 - else - rm "$tmp_file" + # Unset trap if we are not exiting + trap - EXIT fi -else - rm -f "$tmp_file" fi +# The file is either moved or removed, so no explicit rm is needed here. +# The trap handles unexpected exits. ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: The suggestion correctly points out a missing cleanup mechanism for temporary files upon script termination, which is a good shell scripting practice. While this improves robustness, the impact is minor as the container's temporary filesystem is ephemeral. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] More  </td><td></td></tr></tbody></table>