carverauto/serviceradar
1
1
Fork 0
Code Issues 190 Pull requests 16 Projects Releases 173 Packages Wiki Activity Actions

Fix demo LB route regression with declarative Calico /32 config (refs… #3014

Merged
mfreeman451 merged 1 commit from refs/pull/3014/head into staging 2026-03-27 21:42:37 +00:00
Conversation 5 Commits 1 Files changed 3 +81
mfreeman451 commented 2026-03-04 09:22:54 +00:00 (Migrated from github.com)
Owner
Copy link

Imported from GitHub pull request.

Original GitHub pull request: #2992
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2992
Original created: 2026-03-04T09:22:54Z
Original updated: 2026-03-27T21:42:39Z
Original head: carverauto/serviceradar:fix/2991-demo-lb-route-regression
Original base: staging
Original merged: 2026-03-27T21:42:37Z by @mfreeman451

… #2991)

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?
Imported from GitHub pull request. Original GitHub pull request: #2992 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2992 Original created: 2026-03-04T09:22:54Z Original updated: 2026-03-27T21:42:39Z Original head: carverauto/serviceradar:fix/2991-demo-lb-route-regression Original base: staging Original merged: 2026-03-27T21:42:37Z by @mfreeman451 --- … #2991) ## 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?
qodo-code-review[bot] commented 2026-03-04 09:23:09 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2992#issuecomment-3996282604
Original created: 2026-03-04T09:23:09Z

Review Summary by Qodo

Fix demo LB route regression with declarative Calico BGP config

🐞 Bug fix

Grey Divider

Walkthroughs

Description 
• Add Calico BGPConfiguration with explicit /32 service routes
• Include aggregate and individual LoadBalancer IP CIDRs
• Document routing behavior and MetalLB speaker regression
• Update kustomization to include BGP configuration resource
Diagram 
flowchart LR
  A["Calico BGPConfiguration"] -->|"declares serviceLoadBalancerIPs"| B["Explicit /32 routes"]
  B -->|"preferred by FRR"| C["Host routes over connected subnet"]
  D["kustomization.yaml"] -->|"includes"| A
  E["routing1.md"] -->|"documents"| F["MetalLB vs Calico BGP behavior"]
Grey Divider

File Changes

1. k8s/demo/prod/calico-bgpconfiguration.yaml ⚙️ Configuration changes +24/-0

Add Calico BGP configuration with explicit service routes

• New Calico BGPConfiguration resource with AS number 401642
• Defines serviceClusterIPs for internal Kubernetes services
• Declares serviceLoadBalancerIPs with aggregate /27 CIDR and explicit /32 routes
• Includes both IPv4 and IPv6 address ranges for load balancer services

k8s/demo/prod/calico-bgpconfiguration.yaml

2. k8s/demo/prod/kustomization.yaml ⚙️ Configuration changes +1/-0

Include Calico BGP configuration in kustomization

• Add calico-bgpconfiguration.yaml to resources list
• Positioned after service-aliases.demo.yaml and before ingress.yaml

k8s/demo/prod/kustomization.yaml

3. routing1.md 📝 Documentation +56/-0

Document MetalLB vs Calico BGP routing behavior

• Document the root cause of LB route regression with MetalLB speaker disabled
• Explain FRR routing table behavior and connected subnet preference
• Provide two options for resolution with detailed steps
• Include caution notes about VLAN trunking and L3 interface configuration

routing1.md

Grey Divider

Qodo Logo

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2992#issuecomment-3996282604 Original created: 2026-03-04T09:23:09Z --- <h3>Review Summary by Qodo</h3> Fix demo LB route regression with declarative Calico BGP config <code>🐞 Bug fix</code> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <h3>Walkthroughs</h3> <details open> <summary>Description</summary> <br/> <pre> • Add Calico BGPConfiguration with explicit /32 service routes • Include aggregate and individual LoadBalancer IP CIDRs • Document routing behavior and MetalLB speaker regression • Update kustomization to include BGP configuration resource </pre> </details> <details> <summary>Diagram</summary> <br/> > ```mermaid flowchart LR A["Calico BGPConfiguration"] -->|"declares serviceLoadBalancerIPs"| B["Explicit /32 routes"] B -->|"preferred by FRR"| C["Host routes over connected subnet"] D["kustomization.yaml"] -->|"includes"| A E["routing1.md"] -->|"documents"| F["MetalLB vs Calico BGP behavior"] ``` </details> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <h3>File Changes</h3> <details> <summary>1. k8s/demo/prod/calico-bgpconfiguration.yaml <code>⚙️ Configuration changes</code> <code> +24/-0 </code> </summary> <br/> >Add Calico BGP configuration with explicit service routes ><pre> >• New Calico BGPConfiguration resource with AS number 401642 >• Defines serviceClusterIPs for internal Kubernetes services >• Declares serviceLoadBalancerIPs with aggregate /27 CIDR and explicit /32 routes >• Includes both IPv4 and IPv6 address ranges for load balancer services ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2992/files#diff-fdded45f988396353831b2dfb03f29088264cdb2a6a40e25af86a0119ab6986e'> k8s/demo/prod/calico-bgpconfiguration.yaml </a> <hr/> </details> <details> <summary>2. k8s/demo/prod/kustomization.yaml <code>⚙️ Configuration changes</code> <code> +1/-0 </code> </summary> <br/> >Include Calico BGP configuration in kustomization ><pre> >• Add calico-bgpconfiguration.yaml to resources list >• Positioned after service-aliases.demo.yaml and before ingress.yaml ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2992/files#diff-0527e7f19d087f3576d5755a79554797ffbab78b1a7efaa38984b4f3241f6fc9'> k8s/demo/prod/kustomization.yaml </a> <hr/> </details> <details> <summary>3. routing1.md <code>📝 Documentation</code> <code> +56/-0 </code> </summary> <br/> >Document MetalLB vs Calico BGP routing behavior ><pre> >• Document the root cause of LB route regression with MetalLB speaker disabled >• Explain FRR routing table behavior and connected subnet preference >• Provide two options for resolution with detailed steps >• Include caution notes about VLAN trunking and L3 interface configuration ></pre> > ><a href='https://github.com/carverauto/serviceradar/pull/2992/files#diff-d9891b35abbb5d38fb30f09eac6e13bb6e63d7c03c518ebb071db4352d10fdbe'> routing1.md </a> <hr/> </details> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <a href="https://www.qodo.ai"><img src="https://www.qodo.ai/wp-content/uploads/2025/03/qodo-logo.svg" width="80" alt="Qodo Logo"></a>
qodo-code-review[bot] commented 2026-03-04 09:23:10 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2992#issuecomment-3996282687
Original created: 2026-03-04T09:23:10Z

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (1) 📎 Requirement gaps (0)

Grey Divider
Action required
1. routing1.md outside docs 📘 Rule violation ✓ Correctness
Description 
A new operational routing/runbook-style Markdown file was added at the repo root instead of under
docs/docs/. This violates the documentation placement requirement and can lead to documentation
drift and discoverability issues.
Code

routing1.md[1]

+# Routing Notes: MetalLB Speaker vs Calico BGP (demo.serviceradar.cloud)
Evidence 
PR Compliance ID 10 requires new operational runbooks to be placed under docs/docs/. The PR adds
routing1.md (operational routing notes) at the repository root path, not under docs/docs/.

AGENTS.md
routing1.md[1-1]

Agent prompt 
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new operational routing/runbook-style Markdown file was added at the repo root (`routing1.md`), but compliance requires operational runbooks to live under `docs/docs/`.

## Issue Context
This PR introduces routing operational guidance. To keep documentation discoverable and consistent, new operational runbooks must be placed under `docs/docs/`.

## Fix Focus Areas
- routing1.md[1-56]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Kustomize may add namespace 🐞 Bug ⛯ Reliability
Description 
The demo/prod overlay is namespaced but now includes a Calico BGPConfiguration (cluster-scoped CRD).
Because the repo does not include Calico CRDs for kustomize scope inference, kustomize may inject
metadata.namespace, causing kubectl apply -k (used by the repo deploy script) to fail on this
resource and block deployments.
Code

k8s/demo/prod/kustomization.yaml[R8-12]

- ../base
- namespace.yaml
- service-aliases.demo.yaml
+- calico-bgpconfiguration.yaml
- ingress.yaml
Evidence 
The prod kustomization is explicitly namespaced and now includes the new Calico BGPConfiguration
file; the demo deploy script applies this kustomization. The repo demonstrates that CRD scope
information is provided for other cluster-scoped custom kinds (SPIRE) via included CRDs with `scope:
Cluster`, but no analogous Calico CRD exists in this repo to inform kustomize that BGPConfiguration
is cluster-scoped—making namespace injection and apply failure a realistic risk.

k8s/demo/prod/kustomization.yaml[5-14]
k8s/demo/deploy.sh[101-104]
k8s/demo/base/spire/spire-crd-clusterspiffeids.yaml[8-16]
k8s/demo/prod/calico-bgpconfiguration.yaml[1-7]

Agent prompt 
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`k8s/demo/prod` is a namespaced kustomization and now includes a Calico `BGPConfiguration` (cluster-scoped). Kustomize may inject `metadata.namespace` into unknown kinds, which would make the manifest invalid and cause `kubectl apply -k` to fail.

## Issue Context
The repo’s `k8s/demo/deploy.sh` deploy path uses `kubectl apply -k prod/ -n demo`, so a single invalid cluster-scoped object can block the whole demo/prod deployment.

## Fix Focus Areas
- k8s/demo/prod/kustomization.yaml[1-60]
- k8s/demo/prod/calico-bgpconfiguration.yaml[1-24]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. Missing required /32 entry 🐞 Bug ✓ Correctness
Description 
The BGPConfiguration comment says explicit /32s are needed so FRR prefers host routes over the
connected /27, but 23.138.124.24 (used as a static demo LoadBalancer IP) is not included. If Calico
only advertises prefixes listed here, that service IP may remain unreachable when MetalLB speaker is
disabled.
Code

k8s/demo/prod/calico-bgpconfiguration.yaml[R13-22]

+  serviceLoadBalancerIPs:
+    - cidr: 23.138.124.0/27
+    - cidr: 23.138.124.2/32
+    - cidr: 23.138.124.3/32
+    - cidr: 23.138.124.18/32
+    - cidr: 23.138.124.20/32
+    - cidr: 23.138.124.22/32
+    - cidr: 23.138.124.25/32
+    - cidr: 23.138.124.26/32
+    - cidr: 23.138.124.27/32
Evidence 
The new BGPConfiguration explicitly enumerates certain /32s in 23.138.124.0/27 but omits
23.138.124.24/32. Meanwhile, the demo Helm values set a fixed LoadBalancer IP of 23.138.124.24 for
flowgger, making it a concrete IP that should be covered by the intended host-route preference
workaround.

k8s/demo/prod/calico-bgpconfiguration.yaml[11-22]
helm/serviceradar/values-demo.yaml[244-250]

Agent prompt 
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The Calico `serviceLoadBalancerIPs` list is intended to ensure host-route preference, but it omits a statically configured demo LoadBalancer IP (`23.138.124.24`). This can leave at least one externally exposed service without the intended routing behavior.

## Issue Context
`helm/serviceradar/values-demo.yaml` pins flowgger to `23.138.124.24`. The Calico manifest enumerates /32s for other demo IPs but not this one.

## Fix Focus Areas
- k8s/demo/prod/calico-bgpconfiguration.yaml[11-24]
- helm/serviceradar/values-demo.yaml[232-260]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Remediation recommended
4. Overbroad RFC1918 LB CIDR 🐞 Bug ⛯ Reliability
Description 
The BGPConfiguration advertises/permits 192.168.6.0/24 for LoadBalancer IPs, which is far broader
than the repo’s MetalLB pool definition. If this /24 is not exclusively reserved for Kubernetes
service IPs, advertising it can cause unintended routing for addresses outside the actual LB
allocation range.
Code

k8s/demo/prod/calico-bgpconfiguration.yaml[R23-24]

+    - cidr: 2602:f678:0:300::/112
+    - cidr: 192.168.6.0/24
Evidence 
The Calico configuration includes the entire 192.168.6.0/24, while the only MetalLB pool definition
in this repo uses a narrow range (192.168.6.80-192.168.6.95). This mismatch strongly suggests the
Calico scope is broader than necessary for the service IP allocations being configured here.

k8s/demo/prod/calico-bgpconfiguration.yaml[23-24]
k8s/demo/prod/metallb-internal.yaml[1-9]

Agent prompt 
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`192.168.6.0/24` is included under Calico `serviceLoadBalancerIPs`, but the repo’s MetalLB pool (where defined) only allocates 192.168.6.80-192.168.6.95. Advertising/permitting a broader range than actually allocated increases blast radius if the subnet is shared.

## Issue Context
Even if the current environment tolerates this, narrowing to the actual pool reduces risk of unintended routing for addresses that should not be directed at the cluster.

## Fix Focus Areas
- k8s/demo/prod/calico-bgpconfiguration.yaml[13-24]
- k8s/demo/prod/metallb-internal.yaml[1-20]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Grey Divider 
ⓘ The new review experience is currently in Beta. Learn more
Grey Divider

Qodo Logo

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2992#issuecomment-3996282687 Original created: 2026-03-04T09:23:10Z --- <h3>Code Review by Qodo</h3> <code>🐞 Bugs (3)</code> <code>📘 Rule violations (1)</code> <code>📎 Requirement gaps (0)</code> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <br/> <img src="https://www.qodo.ai/wp-content/uploads/2026/01/action-required.png" height="20" alt="Action required"> <details> <summary> 1. <b><i>routing1.md</i></b> outside docs <code>📘 Rule violation</code> <code>✓ Correctness</code></summary> <br/> > <details open> ><summary>Description</summary> ><br/> > ><pre> >A new operational routing/runbook-style Markdown file was added at the repo root instead of under ><b><i>docs/docs/</i></b>. This violates the documentation placement requirement and can lead to documentation >drift and discoverability issues. ></pre> ></details> > <details open> ><summary>Code</summary> ><br/> > ><code>[routing1.md[1]](https://github.com/carverauto/serviceradar/pull/2992/files#diff-d9891b35abbb5d38fb30f09eac6e13bb6e63d7c03c518ebb071db4352d10fdbeR1-R1)</code> > >```diff >+# Routing Notes: MetalLB Speaker vs Calico BGP (demo.serviceradar.cloud) >``` ></details> > <details > ><summary>Evidence</summary> ><br/> > ><pre> >PR Compliance ID 10 requires new operational runbooks to be placed under <b><i>docs/docs/</i></b>. The PR adds ><b><i>routing1.md</i></b> (operational routing notes) at the repository root path, not under <b><i>docs/docs/</i></b>. ></pre> > > <code>AGENTS.md</code> > <code>[routing1.md[1-1]](https://github.com/carverauto/serviceradar/blob/cde0afbc8b7a6e201d5d82a8951d11dbc7618308/routing1.md/#L1-L1)</code> ></details> > <details> ><summary>Agent prompt</summary> ><br/> > >``` >The issue below was found during a code review. Follow the provided context and guidance below and implement a solution > >## Issue description >A new operational routing/runbook-style Markdown file was added at the repo root (`routing1.md`), but compliance requires operational runbooks to live under `docs/docs/`. > >## Issue Context >This PR introduces routing operational guidance. To keep documentation discoverable and consistent, new operational runbooks must be placed under `docs/docs/`. > >## Fix Focus Areas >- routing1.md[1-56] >``` > <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> ></details> <hr/> </details> <details> <summary> 2. Kustomize may add namespace <code>🐞 Bug</code> <code>⛯ Reliability</code></summary> <br/> > <details open> ><summary>Description</summary> ><br/> > ><pre> >The demo/prod overlay is namespaced but now includes a Calico BGPConfiguration (cluster-scoped CRD). >Because the repo does not include Calico CRDs for kustomize scope inference, kustomize may inject ><b><i>metadata.namespace</i></b>, causing <b><i>kubectl apply -k</i></b> (used by the repo deploy script) to fail on this >resource and block deployments. ></pre> ></details> > <details open> ><summary>Code</summary> ><br/> > ><code>[k8s/demo/prod/kustomization.yaml[R8-12]](https://github.com/carverauto/serviceradar/pull/2992/files#diff-0527e7f19d087f3576d5755a79554797ffbab78b1a7efaa38984b4f3241f6fc9R8-R12)</code> > >```diff > - ../base > - namespace.yaml > - service-aliases.demo.yaml >+- calico-bgpconfiguration.yaml > - ingress.yaml >``` ></details> > <details > ><summary>Evidence</summary> ><br/> > ><pre> >The prod kustomization is explicitly namespaced and now includes the new Calico BGPConfiguration >file; the demo deploy script applies this kustomization. The repo demonstrates that CRD scope >information is provided for other cluster-scoped custom kinds (SPIRE) via included CRDs with `scope: >Cluster`, but no analogous Calico CRD exists in this repo to inform kustomize that BGPConfiguration >is cluster-scoped—making namespace injection and apply failure a realistic risk. ></pre> > > <code>[k8s/demo/prod/kustomization.yaml[5-14]](https://github.com/carverauto/serviceradar/blob/cde0afbc8b7a6e201d5d82a8951d11dbc7618308/k8s/demo/prod/kustomization.yaml/#L5-L14)</code> > <code>[k8s/demo/deploy.sh[101-104]](https://github.com/carverauto/serviceradar/blob/cde0afbc8b7a6e201d5d82a8951d11dbc7618308/k8s/demo/deploy.sh/#L101-L104)</code> > <code>[k8s/demo/base/spire/spire-crd-clusterspiffeids.yaml[8-16]](https://github.com/carverauto/serviceradar/blob/cde0afbc8b7a6e201d5d82a8951d11dbc7618308/k8s/demo/base/spire/spire-crd-clusterspiffeids.yaml/#L8-L16)</code> > <code>[k8s/demo/prod/calico-bgpconfiguration.yaml[1-7]](https://github.com/carverauto/serviceradar/blob/cde0afbc8b7a6e201d5d82a8951d11dbc7618308/k8s/demo/prod/calico-bgpconfiguration.yaml/#L1-L7)</code> ></details> > <details> ><summary>Agent prompt</summary> ><br/> > >``` >The issue below was found during a code review. Follow the provided context and guidance below and implement a solution > >## Issue description >`k8s/demo/prod` is a namespaced kustomization and now includes a Calico `BGPConfiguration` (cluster-scoped). Kustomize may inject `metadata.namespace` into unknown kinds, which would make the manifest invalid and cause `kubectl apply -k` to fail. > >## Issue Context >The repo’s `k8s/demo/deploy.sh` deploy path uses `kubectl apply -k prod/ -n demo`, so a single invalid cluster-scoped object can block the whole demo/prod deployment. > >## Fix Focus Areas >- k8s/demo/prod/kustomization.yaml[1-60] >- k8s/demo/prod/calico-bgpconfiguration.yaml[1-24] >``` > <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> ></details> <hr/> </details> <details> <summary> 3. Missing required /32 entry <code>🐞 Bug</code> <code>✓ Correctness</code></summary> <br/> > <details open> ><summary>Description</summary> ><br/> > ><pre> >The BGPConfiguration comment says explicit /32s are needed so FRR prefers host routes over the >connected /27, but 23.138.124.24 (used as a static demo LoadBalancer IP) is not included. If Calico >only advertises prefixes listed here, that service IP may remain unreachable when MetalLB speaker is >disabled. ></pre> ></details> > <details open> ><summary>Code</summary> ><br/> > ><code>[k8s/demo/prod/calico-bgpconfiguration.yaml[R13-22]](https://github.com/carverauto/serviceradar/pull/2992/files#diff-fdded45f988396353831b2dfb03f29088264cdb2a6a40e25af86a0119ab6986eR13-R22)</code> > >```diff >+ serviceLoadBalancerIPs: >+ - cidr: 23.138.124.0/27 >+ - cidr: 23.138.124.2/32 >+ - cidr: 23.138.124.3/32 >+ - cidr: 23.138.124.18/32 >+ - cidr: 23.138.124.20/32 >+ - cidr: 23.138.124.22/32 >+ - cidr: 23.138.124.25/32 >+ - cidr: 23.138.124.26/32 >+ - cidr: 23.138.124.27/32 >``` ></details> > <details > ><summary>Evidence</summary> ><br/> > ><pre> >The new BGPConfiguration explicitly enumerates certain /32s in 23.138.124.0/27 but omits >23.138.124.24/32. Meanwhile, the demo Helm values set a fixed LoadBalancer IP of 23.138.124.24 for >flowgger, making it a concrete IP that should be covered by the intended host-route preference >workaround. ></pre> > > <code>[k8s/demo/prod/calico-bgpconfiguration.yaml[11-22]](https://github.com/carverauto/serviceradar/blob/cde0afbc8b7a6e201d5d82a8951d11dbc7618308/k8s/demo/prod/calico-bgpconfiguration.yaml/#L11-L22)</code> > <code>[helm/serviceradar/values-demo.yaml[244-250]](https://github.com/carverauto/serviceradar/blob/cde0afbc8b7a6e201d5d82a8951d11dbc7618308/helm/serviceradar/values-demo.yaml/#L244-L250)</code> ></details> > <details> ><summary>Agent prompt</summary> ><br/> > >``` >The issue below was found during a code review. Follow the provided context and guidance below and implement a solution > >## Issue description >The Calico `serviceLoadBalancerIPs` list is intended to ensure host-route preference, but it omits a statically configured demo LoadBalancer IP (`23.138.124.24`). This can leave at least one externally exposed service without the intended routing behavior. > >## Issue Context >`helm/serviceradar/values-demo.yaml` pins flowgger to `23.138.124.24`. The Calico manifest enumerates /32s for other demo IPs but not this one. > >## Fix Focus Areas >- k8s/demo/prod/calico-bgpconfiguration.yaml[11-24] >- helm/serviceradar/values-demo.yaml[232-260] >``` > <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> ></details> <hr/> </details> <br/> <img src="https://www.qodo.ai/wp-content/uploads/2026/01/review-recommended.png" height="20" alt="Remediation recommended"> <details> <summary> 4. Overbroad RFC1918 LB CIDR <code>🐞 Bug</code> <code>⛯ Reliability</code></summary> <br/> > <details open> ><summary>Description</summary> ><br/> > ><pre> >The BGPConfiguration advertises/permits <b><i>192.168.6.0/24</i></b> for LoadBalancer IPs, which is far broader >than the repo’s MetalLB pool definition. If this /24 is not exclusively reserved for Kubernetes >service IPs, advertising it can cause unintended routing for addresses outside the actual LB >allocation range. ></pre> ></details> > <details open> ><summary>Code</summary> ><br/> > ><code>[k8s/demo/prod/calico-bgpconfiguration.yaml[R23-24]](https://github.com/carverauto/serviceradar/pull/2992/files#diff-fdded45f988396353831b2dfb03f29088264cdb2a6a40e25af86a0119ab6986eR23-R24)</code> > >```diff >+ - cidr: 2602:f678:0:300::/112 >+ - cidr: 192.168.6.0/24 >``` ></details> > <details > ><summary>Evidence</summary> ><br/> > ><pre> >The Calico configuration includes the entire 192.168.6.0/24, while the only MetalLB pool definition >in this repo uses a narrow range (192.168.6.80-192.168.6.95). This mismatch strongly suggests the >Calico scope is broader than necessary for the service IP allocations being configured here. ></pre> > > <code>[k8s/demo/prod/calico-bgpconfiguration.yaml[23-24]](https://github.com/carverauto/serviceradar/blob/cde0afbc8b7a6e201d5d82a8951d11dbc7618308/k8s/demo/prod/calico-bgpconfiguration.yaml/#L23-L24)</code> > <code>[k8s/demo/prod/metallb-internal.yaml[1-9]](https://github.com/carverauto/serviceradar/blob/cde0afbc8b7a6e201d5d82a8951d11dbc7618308/k8s/demo/prod/metallb-internal.yaml/#L1-L9)</code> ></details> > <details> ><summary>Agent prompt</summary> ><br/> > >``` >The issue below was found during a code review. Follow the provided context and guidance below and implement a solution > >## Issue description >`192.168.6.0/24` is included under Calico `serviceLoadBalancerIPs`, but the repo’s MetalLB pool (where defined) only allocates 192.168.6.80-192.168.6.95. Advertising/permitting a broader range than actually allocated increases blast radius if the subnet is shared. > >## Issue Context >Even if the current environment tolerates this, narrowing to the actual pool reduces risk of unintended routing for addresses that should not be directed at the cluster. > >## Fix Focus Areas >- k8s/demo/prod/calico-bgpconfiguration.yaml[13-24] >- k8s/demo/prod/metallb-internal.yaml[1-20] >``` > <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> ></details> <hr/> </details> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <pre>ⓘ The new review experience is currently in Beta. <a href="https://docs.qodo.ai/qodo-documentation/code-review">Learn more</a></pre> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <!-- https://github.com/carverauto/serviceradar/commit/cde0afbc8b7a6e201d5d82a8951d11dbc7618308 --> <a href="https://www.qodo.ai"><img src="https://www.qodo.ai/wp-content/uploads/2025/03/qodo-logo.svg" width="80" alt="Qodo Logo"></a>
qodo-code-review[bot] commented 2026-03-04 09:28:13 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR review comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2992#discussion_r2882724960
Original created: 2026-03-04T09:28:13Z
Original path: routing1.md
Original line: 1

Action required

1. routing1.md outside docs 📘 Rule violation ✓ Correctness

A new operational routing/runbook-style Markdown file was added at the repo root instead of under
docs/docs/. This violates the documentation placement requirement and can lead to documentation
drift and discoverability issues.
Agent Prompt 
## Issue description
A new operational routing/runbook-style Markdown file was added at the repo root (`routing1.md`), but compliance requires operational runbooks to live under `docs/docs/`.

## Issue Context
This PR introduces routing operational guidance. To keep documentation discoverable and consistent, new operational runbooks must be placed under `docs/docs/`.

## Fix Focus Areas
- routing1.md[1-56]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Imported GitHub PR review comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2992#discussion_r2882724960 Original created: 2026-03-04T09:28:13Z Original path: routing1.md Original line: 1 --- <img src="https://www.qodo.ai/wp-content/uploads/2025/12/v2-action-required.svg" height="20" alt="Action required"> 1\. <b><i>routing1.md</i></b> outside docs <code>📘 Rule violation</code> <code>✓ Correctness</code> <pre> A new operational routing/runbook-style Markdown file was added at the repo root instead of under <b><i>docs/docs/</i></b>. This violates the documentation placement requirement and can lead to documentation drift and discoverability issues. </pre> <details> <summary><strong>Agent Prompt</strong></summary> ``` ## Issue description A new operational routing/runbook-style Markdown file was added at the repo root (`routing1.md`), but compliance requires operational runbooks to live under `docs/docs/`. ## Issue Context This PR introduces routing operational guidance. To keep documentation discoverable and consistent, new operational runbooks must be placed under `docs/docs/`. ## Fix Focus Areas - routing1.md[1-56] ``` <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> </details>
qodo-code-review[bot] commented 2026-03-04 09:28:13 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR review comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2992#discussion_r2882724970
Original created: 2026-03-04T09:28:13Z
Original path: k8s/demo/prod/kustomization.yaml
Original line: 12

Action required

2. Kustomize may add namespace 🐞 Bug ⛯ Reliability

The demo/prod overlay is namespaced but now includes a Calico BGPConfiguration (cluster-scoped CRD).
Because the repo does not include Calico CRDs for kustomize scope inference, kustomize may inject
metadata.namespace, causing kubectl apply -k (used by the repo deploy script) to fail on this
resource and block deployments.
Agent Prompt 
## Issue description
`k8s/demo/prod` is a namespaced kustomization and now includes a Calico `BGPConfiguration` (cluster-scoped). Kustomize may inject `metadata.namespace` into unknown kinds, which would make the manifest invalid and cause `kubectl apply -k` to fail.

## Issue Context
The repo’s `k8s/demo/deploy.sh` deploy path uses `kubectl apply -k prod/ -n demo`, so a single invalid cluster-scoped object can block the whole demo/prod deployment.

## Fix Focus Areas
- k8s/demo/prod/kustomization.yaml[1-60]
- k8s/demo/prod/calico-bgpconfiguration.yaml[1-24]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Imported GitHub PR review comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2992#discussion_r2882724970 Original created: 2026-03-04T09:28:13Z Original path: k8s/demo/prod/kustomization.yaml Original line: 12 --- <img src="https://www.qodo.ai/wp-content/uploads/2025/12/v2-action-required.svg" height="20" alt="Action required"> 2\. Kustomize may add namespace <code>🐞 Bug</code> <code>⛯ Reliability</code> <pre> The demo/prod overlay is namespaced but now includes a Calico BGPConfiguration (cluster-scoped CRD). Because the repo does not include Calico CRDs for kustomize scope inference, kustomize may inject <b><i>metadata.namespace</i></b>, causing <b><i>kubectl apply -k</i></b> (used by the repo deploy script) to fail on this resource and block deployments. </pre> <details> <summary><strong>Agent Prompt</strong></summary> ``` ## Issue description `k8s/demo/prod` is a namespaced kustomization and now includes a Calico `BGPConfiguration` (cluster-scoped). Kustomize may inject `metadata.namespace` into unknown kinds, which would make the manifest invalid and cause `kubectl apply -k` to fail. ## Issue Context The repo’s `k8s/demo/deploy.sh` deploy path uses `kubectl apply -k prod/ -n demo`, so a single invalid cluster-scoped object can block the whole demo/prod deployment. ## Fix Focus Areas - k8s/demo/prod/kustomization.yaml[1-60] - k8s/demo/prod/calico-bgpconfiguration.yaml[1-24] ``` <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> </details>
qodo-code-review[bot] commented 2026-03-04 09:28:13 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR review comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2992#discussion_r2882724984
Original created: 2026-03-04T09:28:13Z
Original path: k8s/demo/prod/calico-bgpconfiguration.yaml
Original line: 22

Action required

3. Missing required /32 entry 🐞 Bug ✓ Correctness

The BGPConfiguration comment says explicit /32s are needed so FRR prefers host routes over the
connected /27, but 23.138.124.24 (used as a static demo LoadBalancer IP) is not included. If Calico
only advertises prefixes listed here, that service IP may remain unreachable when MetalLB speaker is
disabled.
Agent Prompt 
## Issue description
The Calico `serviceLoadBalancerIPs` list is intended to ensure host-route preference, but it omits a statically configured demo LoadBalancer IP (`23.138.124.24`). This can leave at least one externally exposed service without the intended routing behavior.

## Issue Context
`helm/serviceradar/values-demo.yaml` pins flowgger to `23.138.124.24`. The Calico manifest enumerates /32s for other demo IPs but not this one.

## Fix Focus Areas
- k8s/demo/prod/calico-bgpconfiguration.yaml[11-24]
- helm/serviceradar/values-demo.yaml[232-260]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Imported GitHub PR review comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2992#discussion_r2882724984 Original created: 2026-03-04T09:28:13Z Original path: k8s/demo/prod/calico-bgpconfiguration.yaml Original line: 22 --- <img src="https://www.qodo.ai/wp-content/uploads/2025/12/v2-action-required.svg" height="20" alt="Action required"> 3\. Missing required /32 entry <code>🐞 Bug</code> <code>✓ Correctness</code> <pre> The BGPConfiguration comment says explicit /32s are needed so FRR prefers host routes over the connected /27, but 23.138.124.24 (used as a static demo LoadBalancer IP) is not included. If Calico only advertises prefixes listed here, that service IP may remain unreachable when MetalLB speaker is disabled. </pre> <details> <summary><strong>Agent Prompt</strong></summary> ``` ## Issue description The Calico `serviceLoadBalancerIPs` list is intended to ensure host-route preference, but it omits a statically configured demo LoadBalancer IP (`23.138.124.24`). This can leave at least one externally exposed service without the intended routing behavior. ## Issue Context `helm/serviceradar/values-demo.yaml` pins flowgger to `23.138.124.24`. The Calico manifest enumerates /32s for other demo IPs but not this one. ## Fix Focus Areas - k8s/demo/prod/calico-bgpconfiguration.yaml[11-24] - helm/serviceradar/values-demo.yaml[232-260] ``` <code>ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools</code> </details>
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
1week

   
2weeks

   
Failed compliance check

   
IP cameras

   
NATS

NATS JetStream

  
Possible security concern

   
Review effort 1/5

   
Review effort 2/5

   
Review effort 3/5

   
Review effort 4/5

   
Review effort 5/5

   
UI

   
aardvark

   
accessibility

   
amd64

   
api

   
arm64

   
auth

   
back-end

   
bgp

   
blog

   
bug

Something isn't working

  
build

   
checkers

   
ci-cd

continuous integration-continuous deployments

  
cleanup

   
cnpg

cloud-native postgres

  
codex

   
core

core service

  
dependencies

Pull requests that update a dependency file

  
device-management

   
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
dusk

   
ebpf

   
enhancement

New feature or request

  
eta 1d

   
eta 1hr

   
eta 3d

   
eta 3hr

   
feature

   
fieldsurvey

   
github_actions

Pull requests that update GitHub Actions code

  
go

Pull requests that update Go code

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
javascript

Pull requests that update Javascript code

  
k8s

   
log-collector

   
mapper

   
mtr

multi traceroute

  
needs-triage

   
netflow

   
network-sweep

   
observability

   
oracle

Oracle Linux related issues

  
otel

opentelemetry logs, traces, metrics

  
plug-in

   
proton

timeplus proton streaming database

  
python

   
question

Further information is requested

  
reddit

   
redhat

   
research

   
rperf

   
rperf-checker

   
rust

Pull requests that update rust code

  
sdk

   
security

   
serviceradar-agent

   
serviceradar-agent-gateway

   
serviceradar-web

   
serviceradar-web-ng

   
siem

   
snmp

   
sysmon

   
topology

   
ubiquiti

   
wasm

   
wontfix

This will not be worked on

  
zen-engine
No labels
1week
2weeks
Failed compliance check
IP cameras
NATS
Possible security concern
Review effort 1/5
Review effort 2/5
Review effort 3/5
Review effort 4/5
Review effort 5/5
UI
aardvark
accessibility
amd64
api
arm64
auth
back-end
bgp
blog
bug
build
checkers
ci-cd
cleanup
cnpg
codex
core
dependencies
device-management
documentation
duplicate
dusk
ebpf
enhancement
eta 1d
eta 1hr
eta 3d
eta 3hr
feature
fieldsurvey
github_actions
go
good first issue
help wanted
invalid
javascript
k8s
log-collector
mapper
mtr
needs-triage
netflow
network-sweep
observability
oracle
otel
plug-in
proton
python
question
reddit
redhat
research
rperf
rperf-checker
rust
sdk
security
serviceradar-agent
serviceradar-agent-gateway
serviceradar-web
serviceradar-web-ng
siem
snmp
sysmon
topology
ubiquiti
wasm
wontfix
zen-engine
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
carverauto/serviceradar!3014
No description provided.