removing local auth requirement, didnt take in k8s though #2335

Merged

mfreeman451 merged 6 commits from refs/pull/2335/head into main

2025-10-21 21:56:12 +00:00

mfreeman451 commented

2025-10-18 02:15:31 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #1797
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1797
Original created: 2025-10-18T02:15:31Z
Original updated: 2025-10-21T21:56:53Z
Original head: carverauto/serviceradar:update/open_demo_site
Original base: main
Original merged: 2025-10-21T21:56:12Z by @mfreeman451

PR Type

Enhancement, Configuration changes

Description

Disable authentication in Kubernetes demo deployment
Set AUTH_ENABLED to false in both SRQL and Web services
Fix YAML indentation formatting for consistency

Diagram Walkthrough

flowchart LR
  A["K8s Demo Config"] -->|"AUTH_ENABLED: true"| B["Previous State"]
  A -->|"AUTH_ENABLED: false"| C["Updated State"]
  B -->|"SRQL Service"| D["Authentication Required"]
  B -->|"Web Service"| E["Authentication Required"]
  C -->|"SRQL Service"| F["Authentication Disabled"]
  C -->|"Web Service"| G["Authentication Disabled"]

File Walkthrough

Relevant files

Configuration changes

serviceradar-srql.yaml `Disable auth and fix YAML formatting in SRQL` k8s/demo/base/serviceradar-srql.yaml Changed `AUTH_ENABLED` environment variable from `"true"` to `"false"` Fixed YAML indentation formatting for containers, env, ports, volumeMounts, and volumes sections Standardized nested structure indentation throughout the deployment manifest	+69/-69
serviceradar-web.yaml `Disable auth and fix YAML formatting in Web` k8s/demo/base/serviceradar-web.yaml Changed `AUTH_ENABLED` environment variable from `"true"` to `"false"` Fixed YAML indentation formatting for containers, args, ports, env, and service sections Standardized nested structure indentation throughout the deployment manifest	+63/-63

Imported from GitHub pull request. Original GitHub pull request: #1797 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/1797 Original created: 2025-10-18T02:15:31Z Original updated: 2025-10-21T21:56:53Z Original head: carverauto/serviceradar:update/open_demo_site Original base: main Original merged: 2025-10-21T21:56:12Z by @mfreeman451 --- ### **PR Type** Enhancement, Configuration changes ___ ### **Description** - Disable authentication in Kubernetes demo deployment - Set `AUTH_ENABLED` to `false` in both SRQL and Web services - Fix YAML indentation formatting for consistency ___ ### Diagram Walkthrough ```mermaid flowchart LR A["K8s Demo Config"] -->|"AUTH_ENABLED: true"| B["Previous State"] A -->|"AUTH_ENABLED: false"| C["Updated State"] B -->|"SRQL Service"| D["Authentication Required"] B -->|"Web Service"| E["Authentication Required"] C -->|"SRQL Service"| F["Authentication Disabled"] C -->|"Web Service"| G["Authentication Disabled"] ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Configuration changes</strong></td><td><table> <tr> <td> <details> <summary><strong>serviceradar-srql.yaml</strong><dd><code>Disable auth and fix YAML formatting in SRQL</code>                          </dd></summary> <hr> k8s/demo/base/serviceradar-srql.yaml <ul><li>Changed <code>AUTH_ENABLED</code> environment variable from <code>"true"</code> to <code>"false"</code><br> <li> Fixed YAML indentation formatting for containers, env, ports, <br>volumeMounts, and volumes sections<br> <li> Standardized nested structure indentation throughout the deployment <br>manifest</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1797/files#diff-671bb3bfd8bad4c29730ae3c96b9b78e1d446fa378742bd2091b1edad63fec87">+69/-69</a>  </td> </tr> <tr> <td> <details> <summary><strong>serviceradar-web.yaml</strong><dd><code>Disable auth and fix YAML formatting in Web</code>                            </dd></summary> <hr> k8s/demo/base/serviceradar-web.yaml <ul><li>Changed <code>AUTH_ENABLED</code> environment variable from <code>"true"</code> to <code>"false"</code><br> <li> Fixed YAML indentation formatting for containers, args, ports, env, <br>and service sections<br> <li> Standardized nested structure indentation throughout the deployment <br>manifest</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1797/files#diff-38e9af448fa50de08128d5acfd59f72c662a2a89c2da2bd418dc1eedcd6b4679">+63/-63</a>  </td> </tr> </table></td></tr></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2025-10-18 02:15:53 +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/1797#issuecomment-3417712401
Original created: 2025-10-18T02:15:53Z

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Authentication disabled Description: Disabling web authentication via AUTH_ENABLED="false" exposes the demo web service without auth, potentially allowing unauthenticated access to sensitive endpoints. serviceradar-web.yaml [39-60] Referred Code `- name: NODE_ENV value: "production" - name: NEXT_PUBLIC_API_URL value: "http://localhost/api" - name: NEXT_INTERNAL_API_URL value: "http://serviceradar-core:8090" - name: AUTH_ENABLED value: "false" - name: API_KEY valueFrom: secretKeyRef: name: serviceradar-secrets key: api-key - name: JWT_SECRET valueFrom: secretKeyRef: name: serviceradar-secrets key: jwt-secret envFrom: - secretRef: name: serviceradar-secrets ... (clipped 1 lines)`
	Authentication disabled Description: Disabling SRQL authentication via AUTH_ENABLED="false" while keeping SRQL_REQUIRE_BEARER="true" may create inconsistent or misleading security posture and allow unauthenticated access if bearer checks are bypassed. serviceradar-srql.yaml [33-37] Referred Code `- name: AUTH_ENABLED value: "false" - name: SRQL_REQUIRE_BEARER value: "true" - name: PROTON_PASSWORD`
Ticket Compliance
⚪	🎫 No ticket provided `- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true --> </details></td></tr>`
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.
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/1797#issuecomment-3417712401 Original created: 2025-10-18T02:15:53Z --- _You are nearing your monthly Qodo Merge usage quota. For more information, please visit [here](https://qodo-merge-docs.qodo.ai/installation/qodo_merge/#cloud-users)._ ## 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=2>⚪</td> <td><details><summary><strong>Authentication disabled </strong></summary><br> <b>Description:</b> Disabling web authentication via AUTH_ENABLED="false" exposes the demo web service without <br>auth, potentially allowing unauthenticated access to sensitive endpoints.<br> <strong><a href='https://github.com/carverauto/serviceradar/pull/1797/files#diff-38e9af448fa50de08128d5acfd59f72c662a2a89c2da2bd418dc1eedcd6b4679R39-R60'>serviceradar-web.yaml [39-60]</a></strong><br> <details open><summary>Referred Code</summary> ```yaml - name: NODE_ENV value: "production" - name: NEXT_PUBLIC_API_URL value: "http://localhost/api" - name: NEXT_INTERNAL_API_URL value: "http://serviceradar-core:8090" - name: AUTH_ENABLED value: "false" - name: API_KEY valueFrom: secretKeyRef: name: serviceradar-secrets key: api-key - name: JWT_SECRET valueFrom: secretKeyRef: name: serviceradar-secrets key: jwt-secret envFrom: - secretRef: name: serviceradar-secrets ... (clipped 1 lines) ``` </details></details></td></tr> <tr><td><details><summary><strong>Authentication disabled </strong></summary><br> <b>Description:</b> Disabling SRQL authentication via AUTH_ENABLED="false" while keeping <br>SRQL_REQUIRE_BEARER="true" may create inconsistent or misleading security posture and <br>allow unauthenticated access if bearer checks are bypassed.<br> <strong><a href='https://github.com/carverauto/serviceradar/pull/1797/files#diff-671bb3bfd8bad4c29730ae3c96b9b78e1d446fa378742bd2091b1edad63fec87R33-R37'>serviceradar-srql.yaml [33-37]</a></strong><br> <details open><summary>Referred Code</summary> ```yaml - name: AUTH_ENABLED value: "false" - name: SRQL_REQUIRE_BEARER value: "true" - name: PROTON_PASSWORD ``` </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"> - [ ] Update  </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-18 02:17:01 +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/1797#issuecomment-3417713150
Original created: 2025-10-18T02:17:01Z

PR Code Suggestions ✨

Latest suggestions up to 4518b37

Category	Suggestion	Impact
Incremental ^[*]	Disable SRQL auth in demo In `serviceradar-srql.yaml`, set the `AUTH_ENABLED` and `SRQL_REQUIRE_BEARER` environment variables to `"false"` to align with the PR's goal of disabling authentication. k8s/demo/base/serviceradar-srql.yaml [33-36] `- name: AUTH_ENABLED - value: "true" + value: "false" - name: SRQL_REQUIRE_BEARER - value: "true" + value: "false"` Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies an inconsistency where `AUTH_ENABLED` was set to `false` in `serviceradar-web.yaml` but not in `serviceradar-srql.yaml`, which would likely cause authentication issues against the PR's intent.	Medium
Incremental ^[*]	Preserve client source IP for gRPC Add `externalTrafficPolicy: Local` to the `serviceradar-core-grpc-external` service specification to preserve the client's source IP address. k8s/demo/prod/serviceradar-core-grpc-external.yaml [7-21] `annotations: metallb.universe.tf/address-pool: k3s-pool metallb.universe.tf/allow-shared-ip: serviceradar-core spec: type: LoadBalancer + externalTrafficPolicy: Local selector: app: serviceradar-core app.kubernetes.io/part-of: serviceradar loadBalancerSourceRanges: - 192.168.6.87/32 ports: - name: grpc port: 50052 protocol: TCP targetPort: grpc` Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion provides a valid and beneficial improvement by recommending `externalTrafficPolicy: Local` to preserve the client source IP, which is good practice for `LoadBalancer` services, especially when `loadBalancerSourceRanges` is used.	Low
Possible issue	Use a stable health endpoint Change the liveness and readiness probes for `serviceradar-web` to use a stable, unauthenticated endpoint like `/health` instead of `/login`, since `AUTH_ENABLED` is now `false`. k8s/demo/base/serviceradar-web.yaml [45-82] `- name: AUTH_ENABLED value: "false" ... livenessProbe: httpGet: - path: /login + path: /health port: 3000 initialDelaySeconds: 30 periodSeconds: 30 timeoutSeconds: 10 failureThreshold: 3 readinessProbe: httpGet: - path: /login + path: /health port: 3000 initialDelaySeconds: 10 periodSeconds: 10 timeoutSeconds: 5 failureThreshold: 3` `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 7 __ Why: This is a valid suggestion that improves the robustness of the health checks, as disabling authentication might change the behavior of the `/login` endpoint, potentially causing false negatives.	Medium
Possible issue	Use explicit gRPC targetPort In the `serviceradar-core-grpc-external` service, change `targetPort` from the name `grpc` to the explicit port number `50052` to ensure correct traffic routing. k8s/demo/prod/serviceradar-core-grpc-external.yaml [4-21] `-metadata: - name: serviceradar-core-grpc-external - namespace: demo - annotations: - metallb.universe.tf/address-pool: k3s-pool - metallb.universe.tf/allow-shared-ip: serviceradar-core spec: type: LoadBalancer selector: app: serviceradar-core app.kubernetes.io/part-of: serviceradar loadBalancerSourceRanges: - 192.168.6.87/32 ports: - name: grpc port: 50052 protocol: TCP - targetPort: grpc + targetPort: 50052` Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a potential misconfiguration where using a named `targetPort` relies on the Pod definition. Using an explicit port number makes the Service definition more robust and self-contained.	Low
More

Previous suggestions

Suggestions up to commit 3a8d4e2

Category	Suggestion	Impact
Possible issue	Update probes for disabled authentication Update the `livenessProbe` and `readinessProbe` to use a dedicated health endpoint (e.g., `/healthz`) instead of `/login`, as the login page may be unavailable when authentication is disabled. k8s/demo/base/serviceradar-web.yaml [67-82] `livenessProbe: httpGet: - path: /login + path: /healthz port: 3000 ... readinessProbe: httpGet: - path: /login + path: /healthz port: 3000` Suggestion importance[1-10]: 10 __ Why: This suggestion correctly identifies a critical issue where disabling authentication would likely cause the health probes to fail, preventing the service from becoming ready and leading to deployment failure.	High
Possible issue	Align bearer token requirement with authentication Set the `SRQL_REQUIRE_BEARER` environment variable to `"false"` to align with `AUTH_ENABLED` being disabled, preventing a configuration conflict. k8s/demo/base/serviceradar-srql.yaml [33-36] `- name: AUTH_ENABLED value: "false" - name: SRQL_REQUIRE_BEARER - value: "true" + value: "false"` Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a logical configuration conflict between `AUTH_ENABLED` and `SRQL_REQUIRE_BEARER` that would likely cause runtime errors, making this a critical fix.	High

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1797#issuecomment-3417713150 Original created: 2025-10-18T02:17:01Z --- ## PR Code Suggestions ✨  Latest suggestions up to 4518b37 <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=2>Incremental <sup><a href='https://qodo-merge-docs.qodo.ai/core-abilities/incremental_update/'>[*]</a></sup></td> <td> <details><summary>Disable SRQL auth in demo</summary> ___ **In <code>serviceradar-srql.yaml</code>, set the <code>AUTH_ENABLED</code> and <code>SRQL_REQUIRE_BEARER</code> <br>environment variables to <code>"false"</code> to align with the PR's goal of disabling <br>authentication.** [k8s/demo/base/serviceradar-srql.yaml [33-36]](https://github.com/carverauto/serviceradar/pull/1797/files#diff-671bb3bfd8bad4c29730ae3c96b9b78e1d446fa378742bd2091b1edad63fec87R33-R36) ```diff - name: AUTH_ENABLED - value: "true" + value: "false" - name: SRQL_REQUIRE_BEARER - value: "true" + value: "false" ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies an inconsistency where `AUTH_ENABLED` was set to `false` in `serviceradar-web.yaml` but not in `serviceradar-srql.yaml`, which would likely cause authentication issues against the PR's intent. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Preserve client source IP for gRPC</summary> ___ **Add <code>externalTrafficPolicy: Local</code> to the <code>serviceradar-core-grpc-external</code> service <br>specification to preserve the client's source IP address.** [k8s/demo/prod/serviceradar-core-grpc-external.yaml [7-21]](https://github.com/carverauto/serviceradar/pull/1797/files#diff-67e2a1dffbc8d92de3c92174fd842cf8d8591ed6e8e60cbeff3e726090656d49R7-R21) ```diff annotations: metallb.universe.tf/address-pool: k3s-pool metallb.universe.tf/allow-shared-ip: serviceradar-core spec: type: LoadBalancer + externalTrafficPolicy: Local selector: app: serviceradar-core app.kubernetes.io/part-of: serviceradar loadBalancerSourceRanges: - 192.168.6.87/32 ports: - name: grpc port: 50052 protocol: TCP targetPort: grpc ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: The suggestion provides a valid and beneficial improvement by recommending `externalTrafficPolicy: Local` to preserve the client source IP, which is good practice for `LoadBalancer` services, especially when `loadBalancerSourceRanges` is used. </details></details></td><td align=center>Low </td></tr><tr><td rowspan=2>Possible issue</td> <td> <details><summary>Use a stable health endpoint</summary> ___ **Change the liveness and readiness probes for <code>serviceradar-web</code> to use a stable, <br>unauthenticated endpoint like <code>/health</code> instead of <code>/login</code>, since <code>AUTH_ENABLED</code> is <br>now <code>false</code>.** [k8s/demo/base/serviceradar-web.yaml [45-82]](https://github.com/carverauto/serviceradar/pull/1797/files#diff-38e9af448fa50de08128d5acfd59f72c662a2a89c2da2bd418dc1eedcd6b4679R45-R82) ```diff - name: AUTH_ENABLED value: "false" ... livenessProbe: httpGet: - path: /login + path: /health port: 3000 initialDelaySeconds: 30 periodSeconds: 30 timeoutSeconds: 10 failureThreshold: 3 readinessProbe: httpGet: - path: /login + path: /health port: 3000 initialDelaySeconds: 10 periodSeconds: 10 timeoutSeconds: 5 failureThreshold: 3 ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: This is a valid suggestion that improves the robustness of the health checks, as disabling authentication might change the behavior of the `/login` endpoint, potentially causing false negatives. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Use explicit gRPC targetPort</summary> ___ **In the <code>serviceradar-core-grpc-external</code> service, change <code>targetPort</code> from the name <br><code>grpc</code> to the explicit port number <code>50052</code> to ensure correct traffic routing.** [k8s/demo/prod/serviceradar-core-grpc-external.yaml [4-21]](https://github.com/carverauto/serviceradar/pull/1797/files#diff-67e2a1dffbc8d92de3c92174fd842cf8d8591ed6e8e60cbeff3e726090656d49R4-R21) ```diff -metadata: - name: serviceradar-core-grpc-external - namespace: demo - annotations: - metallb.universe.tf/address-pool: k3s-pool - metallb.universe.tf/allow-shared-ip: serviceradar-core spec: type: LoadBalancer selector: app: serviceradar-core app.kubernetes.io/part-of: serviceradar loadBalancerSourceRanges: - 192.168.6.87/32 ports: - name: grpc port: 50052 protocol: TCP - targetPort: grpc + targetPort: 50052 ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: The suggestion correctly identifies a potential misconfiguration where using a named `targetPort` relies on the Pod definition. Using an explicit port number makes the Service definition more robust and self-contained. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] More  </td><td></td></tr></tbody></table> ___ #### Previous suggestions <details><summary>Suggestions up to commit 3a8d4e2</summary> <br><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=2>Possible issue</td> <td> <details><summary>Update probes for disabled authentication</summary> ___ **Update the <code>livenessProbe</code> and <code>readinessProbe</code> to use a dedicated health endpoint <br>(e.g., <code>/healthz</code>) instead of <code>/login</code>, as the login page may be unavailable when <br>authentication is disabled.** [k8s/demo/base/serviceradar-web.yaml [67-82]](https://github.com/carverauto/serviceradar/pull/1797/files#diff-38e9af448fa50de08128d5acfd59f72c662a2a89c2da2bd418dc1eedcd6b4679R67-R82) ```diff livenessProbe: httpGet: - path: /login + path: /healthz port: 3000 ... readinessProbe: httpGet: - path: /login + path: /healthz port: 3000 ``` <details><summary>Suggestion importance[1-10]: 10</summary> __ Why: This suggestion correctly identifies a critical issue where disabling authentication would likely cause the health probes to fail, preventing the service from becoming ready and leading to deployment failure. </details></details></td><td align=center>High </td></tr><tr><td> <details><summary>Align bearer token requirement with authentication</summary> ___ **Set the <code>SRQL_REQUIRE_BEARER</code> environment variable to <code>"false"</code> to align with <br><code>AUTH_ENABLED</code> being disabled, preventing a configuration conflict.** [k8s/demo/base/serviceradar-srql.yaml [33-36]](https://github.com/carverauto/serviceradar/pull/1797/files#diff-671bb3bfd8bad4c29730ae3c96b9b78e1d446fa378742bd2091b1edad63fec87R33-R36) ```diff - name: AUTH_ENABLED value: "false" - name: SRQL_REQUIRE_BEARER - value: "true" + value: "false" ```  <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: The suggestion correctly identifies a logical configuration conflict between `AUTH_ENABLED` and `SRQL_REQUIRE_BEARER` that would likely cause runtime errors, making this a critical fix. </details></details></td><td align=center>High </td></tr> <tr><td align="center" colspan="2">  </td><td></td></tr></tbody></table> </details>